Re: [PATCH 1/2] rtc: max77686: Add support for MAX20024/MAX77620 RTC IP

[Laxman Dewangan]

On Wednesday 02 March 2016 09:22 AM, Krzysztof Kozlowski wrote:
> On 02.03.2016 11:15, Laxman Dewangan wrote:
>
> > > > (kernel_ulong_t)&max77802_drv_data, },
> > > > +    { "max77620-rtc", .driver_data =
> > > > (kernel_ulong_t)&max77620_drv_data, },
> > > > +    { "max20024-rtc", .driver_data =
> > > > (kernel_ulong_t)&max77620_drv_data, },
> > > There shouldn't be "max20024-rtc". This is exactly the same as
> > > "max77620-rtc" so re-use existing id. No point of duplicating device
> > > names for 100% compatible devices.
> > I am thinking that having compatible for each device which it supports
> > is better.
> >
> > In MFD, I have made all sub module of max20024 as max20024-<module>.
> > I have not mixed the sub module name for max20024 with max77620 module.
> The point of compatible is to be... compatible so you don't create
> compatibles for the same meaning!
>
> However this is actually not a compatible but a matching name... which
> should follow the same idea. You did not give any argument why this is
> better.

My point is that if any driver supporting the any devices then it should 
be there in their compatibility although other everything is same.

This way, it is easy to find that the driver is available for the device 
or not. Also easy way to tell that someone has invested time to find out 
the driver corresponding to device and he confirmed that this driver is 
compatible with that device.
Otherwise, it is difficult to quickly find out the driver whether this 
is available/support or not for given device.

Datasheet never says that this device is same as some other device and 
hence this is the only place to tell for SW guys.



> For me, code like this:
> { "max77802-rtc", .driver_data = (kernel_ulong_t)&max77802_drv_data, },
> { "max77620-rtc", .driver_data = (kernel_ulong_t)&max77620_drv_data, },
> { "max77621-rtc", .driver_data = (kernel_ulong_t)&max77620_drv_data, },
> { "max77622-rtc", .driver_data = (kernel_ulong_t)&max77620_drv_data, },
> { "max77623-rtc", .driver_data = (kernel_ulong_t)&max77620_drv_data, },
> { "max776xx-some-other-rtc", .driver_data =
> (kernel_ulong_t)&max77620_drv_data, },
> { "max77624-rtc", .driver_data = (kernel_ulong_t)&max77620_drv_data, },
>
> is useless, ridiculous and obfuscated. It is duplication of code just
> "because". The child driver is selected by matching mfd-cell or
> compatible. We are reusing child drivers so reuse under the same name.
>
> Best regards,
> Krzysztof