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

[Chen-Yu Tsai]

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? Hans has made a good
argument, so I take back any doubts I raised.

ChenYu