[linux-sunxi] [PATCH v2 04/13] rc: sunxi-cir: Add support for an optional reset controller

[Chen-Yu Tsai]

On Mon, Jan 19, 2015 at 10:17 PM, Hans de Goede <hdegoede at redhat.com> wrote:
> Hi,
>
>
> On 19-01-15 15:10, Chen-Yu Tsai wrote:
>>
>> Hi,
>>
>> On Sat, Dec 20, 2014 at 6:20 PM, Hans de Goede <hdegoede at redhat.com>
>> wrote:
>>>
>>> Hi,
>>>
>>>
>>> On 19-12-14 19:17, Maxime Ripard wrote:
>>>>
>>>>
>>>> Hi,
>>>>
>>>> On Thu, Dec 18, 2014 at 09:50:26AM +0100, Hans de Goede wrote:
>>>>>
>>>>>
>>>>> Hi,
>>>>>
>>>>> On 18-12-14 03:48, Chen-Yu Tsai wrote:
>>>>>>
>>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> On Thu, Dec 18, 2014 at 1:18 AM, Hans de Goede <hdegoede at redhat.com>
>>>>>> wrote:
>>>>>>>
>>>>>>>
>>>>>>> On sun6i the cir block is attached to the reset controller, add
>>>>>>> support
>>>>>>> for de-asserting the reset if a reset controller is specified in dt.
>>>>>>>
>>>>>>> Signed-off-by: Hans de Goede <hdegoede at redhat.com>
>>>>>>> Acked-by: Mauro Carvalho Chehab <mchehab at osg.samsung.com>
>>>>>>> Acked-by: Maxime Ripard <maxime.ripard at free-electrons.com>
>>>>>>> ---
>>>>>>>    .../devicetree/bindings/media/sunxi-ir.txt         |  2 ++
>>>>>>>    drivers/media/rc/sunxi-cir.c                       | 25
>>>>>>> ++++++++++++++++++++--
>>>>>>>    2 files changed, 25 insertions(+), 2 deletions(-)
>>>>>>>
>>>>>>> diff --git a/Documentation/devicetree/bindings/media/sunxi-ir.txt
>>>>>>> b/Documentation/devicetree/bindings/media/sunxi-ir.txt
>>>>>>> index 23dd5ad..6b70b9b 100644
>>>>>>> --- a/Documentation/devicetree/bindings/media/sunxi-ir.txt
>>>>>>> +++ b/Documentation/devicetree/bindings/media/sunxi-ir.txt
>>>>>>> @@ -10,6 +10,7 @@ Required properties:
>>>>>>>
>>>>>>>    Optional properties:
>>>>>>>    - linux,rc-map-name : Remote control map name.
>>>>>>> +- resets : phandle + reset specifier pair
>>>>>>
>>>>>>
>>>>>>
>>>>>> Should it be optional? Or should we use a sun6i compatible with
>>>>>> a mandatory reset phandle? I mean, the driver/hardware is not
>>>>>> going to work with the reset missing on sun6i.
>>>>>>
>>>>>> Seems we are doing it one way for some of our drivers, and
>>>>>> the other (optional) way for more generic ones, like USB.
>>>>>
>>>>>
>>>>>
>>>>> I do not believe that we should add a new compatible just because
>>>>> the reset line of a block is hooked up differently. It is the
>>>>> exact same ip-block. Only now the reset is not controlled
>>>>> through the apb-gate, but controlled separately.
>>>>
>>>>
>>>>
>>>> He has a point though. Your driver might very well probe nicely and
>>>> everything, but still wouldn't be functional at all because the reset
>>>> line wouldn't have been specified in the DT.
>>>
>>>
>>>
>>> Right, just like other drivers we've, see e.g.:
>>>
>>> Documentation/devicetree/bindings/mmc/sunxi-mmc.txt
>>>
>>> Which is dealing with this in the same way.
>>>
>>>> The easiest way to deal with that would be in the bindings doc to
>>>> update it with a compatible for the A31, and mentionning that the
>>>> reset property is mandatory there.
>>>
>>>
>>>
>>> No the easiest way to deal with this is to expect people writing
>>> the dts to know what they are doing, just like we do for a lot
>>> of the other blocks in sun6i.
>>>
>>> Maybe put a generic note somewhere that sun6i has a reset controller,
>>> and that for all the blocks with optional resets property it should
>>> be considered mandatory on sun6i ?
>>>
>>> I'm sorry but I'm not going to make this change for the ir bindings
>>> given that we've the same situation in a lot of other places.
>>>
>>> Consistency is important. Moreover I believe that having a sun6i
>>> specific compatible string is just wrong, since it is the exact
>>> same hardware block as on sun5i, just with its reset line routed
>>> differently, just like e.g. the mmc controller, the uarts or the gmac
>>> all of which also do not have a sun6i specific compatible to enforce
>>> reset controller usage.
>>>
>>> Regards,
>>>
>>> Hans
>>>
>>>
>>>
>>>
>>>> Note that the code itself might not change at all though. I'd just
>>>> like to avoid any potential breaking of the DT bindings themselves. If
>>>> we further want to refine the code, we can do that however we want.
>>>>
>>>> I have a slight preference for a clean error if reset is missing, but
>>>> I won't get in the way just for that.
>>
>>
>> Seems this patch and the following patch were overlooked after the
>> discussion. Any chance we could get this in?
>
>
> I'm a linux/media sub-maintainer, so I've already send a pull-req for
> these 2 to the linux/media maintainer, iow this is taken care of :)

That's good to hear. I was going through the mainlining effort page,
and couldn't find these 2 in linux-next. I'll mark them as planned
for 3.20 then.


Chen-Yu