Re: [PATCH v2] i2c: core: make it possible to match a pure device tree driver

Re: [PATCH v2] i2c: core: make it possible to match a pure device tree driver

[Stephen Warren]

On 05/22/2013 01:56 AM, Linus Walleij wrote:
> On Mon, May 13, 2013 at 10:18 PM, Linus Walleij
> <linus.walleij@linaro.org> wrote:
> 
>> This tries to address an issue found when writing an MFD driver
>> for the Nomadik STw481x PMICs: as the platform is using device
>> tree exclusively I want to specify the driver matching like
>> this:
> (...)
>> ---
>> ChangeLog v1->v2:
>> - Use of_match_device() to determine if there is a DT match in
>>   the probe code. If there is a match we pass NULL for the
>>   id_table match parameter.
> 
> Ping on this.
> 
> v2 should be doing what Grant suggested...

This has just shown up in next-20130617, and breaks at least the
TPS65910 and TPS62360 drivers, since they assume that the id parameter
passed to probe is non-NULL. However, now the parameter is NULL since
these drivers have both an ID table and an OF match table.

I'd like to suggest this patch be reverted an re-introduced immediately
after the merge window. That should give enough time for everyone to get
a heads-up on fixing any drivers with a similar problem, rather than
trying to cram all that in immediately before the merge window. I'd also
suggest that this patch should be accompanied by fixes for any similarly
broken drivers, based on code inspection.

Do people agree? If not, please let us know ASAP so we can post patches
to fix this.

Re: [PATCH v2] i2c: core: make it possible to match a pure device tree driver

[Linus Walleij]

On Mon, Jun 17, 2013 at 5:48 PM, Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> wrote:

> This has just shown up in next-20130617, and breaks at least the
> TPS65910 and TPS62360 drivers, since they assume that the id parameter
> passed to probe is non-NULL. However, now the parameter is NULL since
> these drivers have both an ID table and an OF match table.

So you mean they come in through the DT boot path and assume
that parameter is non-null even though they should not make use of
it?

> I'd like to suggest this patch be reverted an re-introduced immediately
> after the merge window. That should give enough time for everyone to get
> a heads-up on fixing any drivers with a similar problem, rather than
> trying to cram all that in immediately before the merge window.

OK that works for me, I'm not in any hurry.

Wolfram get to decide how to handle this...

Yours,
Linus Walleij

Re: [PATCH v2] i2c: core: make it possible to match a pure device tree driver

[Stephen Warren]

On 06/17/2013 10:33 AM, Linus Walleij wrote:
> On Mon, Jun 17, 2013 at 5:48 PM, Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> wrote:
> 
>> This has just shown up in next-20130617, and breaks at least the
>> TPS65910 and TPS62360 drivers, since they assume that the id parameter
>> passed to probe is non-NULL. However, now the parameter is NULL since
>> these drivers have both an ID table and an OF match table.
> 
> So you mean they come in through the DT boot path and assume
> that parameter is non-null even though they should not make use of
> it?

Yes. Since this wasn't true/enforced before, it probably wasn't clear
that rule existed.

Re: [PATCH v2] i2c: core: make it possible to match a pure device tree driver

[Grant Likely]

On Mon, Jun 17, 2013 at 5:33 PM, Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> On Mon, Jun 17, 2013 at 5:48 PM, Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> wrote:
>
>> This has just shown up in next-20130617, and breaks at least the
>> TPS65910 and TPS62360 drivers, since they assume that the id parameter
>> passed to probe is non-NULL. However, now the parameter is NULL since
>> these drivers have both an ID table and an OF match table.
>
> So you mean they come in through the DT boot path and assume
> that parameter is non-null even though they should not make use of
> it?
>
>> I'd like to suggest this patch be reverted an re-introduced immediately
>> after the merge window. That should give enough time for everyone to get
>> a heads-up on fixing any drivers with a similar problem, rather than
>> trying to cram all that in immediately before the merge window.
>
> OK that works for me, I'm not in any hurry.

Deferring by a merge window isn't going to make it any less painful.
Do your best to find all the users that need to be changed. Use a
coccinelle search perhaps, but I think it should be merged anyway.

g.

Re: [PATCH v2] i2c: core: make it possible to match a pure device tree driver

[Stephen Warren]

On 06/17/2013 04:15 PM, Grant Likely wrote:
> On Mon, Jun 17, 2013 at 5:33 PM, Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
>> On Mon, Jun 17, 2013 at 5:48 PM, Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> wrote:
>>
>>> This has just shown up in next-20130617, and breaks at least the
>>> TPS65910 and TPS62360 drivers, since they assume that the id parameter
>>> passed to probe is non-NULL. However, now the parameter is NULL since
>>> these drivers have both an ID table and an OF match table.
>>
>> So you mean they come in through the DT boot path and assume
>> that parameter is non-null even though they should not make use of
>> it?
>>
>>> I'd like to suggest this patch be reverted an re-introduced immediately
>>> after the merge window. That should give enough time for everyone to get
>>> a heads-up on fixing any drivers with a similar problem, rather than
>>> trying to cram all that in immediately before the merge window.
>>
>> OK that works for me, I'm not in any hurry.
> 
> Deferring by a merge window isn't going to make it any less painful.

It'll give a lot more time for people to be exposed to the change and
hence find/fix it linux-next rather than seeing it for the first time in
Linus's tree.

Re: [PATCH v2] i2c: core: make it possible to match a pure device tree driver

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 1317 bytes --]

On Mon, Jun 17, 2013 at 11:15:30PM +0100, Grant Likely wrote:
> On Mon, Jun 17, 2013 at 5:33 PM, Linus Walleij <linus.walleij@linaro.org> wrote:
> > On Mon, Jun 17, 2013 at 5:48 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> >
> >> This has just shown up in next-20130617, and breaks at least the
> >> TPS65910 and TPS62360 drivers, since they assume that the id parameter
> >> passed to probe is non-NULL. However, now the parameter is NULL since
> >> these drivers have both an ID table and an OF match table.
> >
> > So you mean they come in through the DT boot path and assume
> > that parameter is non-null even though they should not make use of
> > it?
> >
> >> I'd like to suggest this patch be reverted an re-introduced immediately
> >> after the merge window. That should give enough time for everyone to get
> >> a heads-up on fixing any drivers with a similar problem, rather than
> >> trying to cram all that in immediately before the merge window.
> >
> > OK that works for me, I'm not in any hurry.
> 
> Deferring by a merge window isn't going to make it any less painful.
> Do your best to find all the users that need to be changed. Use a
> coccinelle search perhaps, but I think it should be merged anyway.

I'll try a bit of my coccinelle-foo today and then decide.


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH v2] i2c: core: make it possible to match a pure device tree driver

[Linus Walleij]

On Tue, Jun 18, 2013 at 9:33 AM, Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org> wrote:
> On Mon, Jun 17, 2013 at 11:15:30PM +0100, Grant Likely wrote:
>> On Mon, Jun 17, 2013 at 5:33 PM, Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
>> > OK that works for me, I'm not in any hurry.
>>
>> Deferring by a merge window isn't going to make it any less painful.
>> Do your best to find all the users that need to be changed. Use a
>> coccinelle search perhaps, but I think it should be merged anyway.
>
> I'll try a bit of my coccinelle-foo today and then decide.

Thanks Wolfram, much appreciated.

Yours,
Linus Walleij

Re: [PATCH v2] i2c: core: make it possible to match a pure device tree driver

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 1202 bytes --]

On Tue, Jun 18, 2013 at 09:44:17AM +0200, Linus Walleij wrote:
> On Tue, Jun 18, 2013 at 9:33 AM, Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org> wrote:
> > On Mon, Jun 17, 2013 at 11:15:30PM +0100, Grant Likely wrote:
> >> On Mon, Jun 17, 2013 at 5:33 PM, Linus Walleij <linus.walleij@linaro.org> wrote:
> >> > OK that works for me, I'm not in any hurry.
> >>
> >> Deferring by a merge window isn't going to make it any less painful.
> >> Do your best to find all the users that need to be changed. Use a
> >> coccinelle search perhaps, but I think it should be merged anyway.
> >
> > I'll try a bit of my coccinelle-foo today and then decide.
> 
> Thanks Wolfram, much appreciated.

I am going to revert that commit. I was thinking back and forth, even
playing with the idea to remove the id as a parameter to probe for i2c
drivers and let them request the id from the i2c core when needed. But
now I found more side-effects. E.g. run-time based instantiation for i2c
devices is depending on an id-table. So, for now I keep insisting that
an id-table must exist. Looks like DT-only drivers need more thinking
and this is too late for 3.11.

Regards,

   Wolfram


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]