Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1210207 > unrolled thread

[PATCH 13/18] iio: adc: mcp320x: Set struct spi_driver .of_match_table

Started byJavier Martinez Canillas <javier@osg.samsung.com>
First post2015-08-20 09:10 +0200
Last post2015-08-24 09:20 +0200
Articles 10 — 4 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  [PATCH 13/18] iio: adc: mcp320x: Set struct spi_driver .of_match_table Javier Martinez Canillas <javier@osg.samsung.com> - 2015-08-20 09:10 +0200
    Re: [PATCH 13/18] iio: adc: mcp320x: Set struct spi_driver  .of_match_table Michael Welling <mwelling@ieee.org> - 2015-08-20 22:10 +0200
      Re: [PATCH 13/18] iio: adc: mcp320x: Set struct spi_driver  .of_match_table Javier Martinez Canillas <javier@osg.samsung.com> - 2015-08-21 00:10 +0200
        Re: [PATCH 13/18] iio: adc: mcp320x: Set struct spi_driver  .of_match_table Michael Welling <mwelling@ieee.org> - 2015-08-21 00:30 +0200
          Re: [PATCH 13/18] iio: adc: mcp320x: Set struct spi_driver  .of_match_table Javier Martinez Canillas <javier@osg.samsung.com> - 2015-08-21 00:50 +0200
            Re: [PATCH 13/18] iio: adc: mcp320x: Set struct spi_driver  .of_match_table Michael Welling <mwelling@ieee.org> - 2015-08-21 02:00 +0200
              Re: [PATCH 13/18] iio: adc: mcp320x: Set struct spi_driver  .of_match_table Jonathan Cameron <jic23@kernel.org> - 2015-08-22 20:10 +0200
            Re: [PATCH 13/18] iio: adc: mcp320x: Set struct spi_driver  .of_match_table Jonathan Cameron <jic23@kernel.org> - 2015-08-22 19:50 +0200
              Re: [PATCH 13/18] iio: adc: mcp320x: Set struct spi_driver .of_match_table Rob Herring <robherring2@gmail.com> - 2015-08-24 00:20 +0200
                Re: [PATCH 13/18] iio: adc: mcp320x: Set struct spi_driver  .of_match_table Javier Martinez Canillas <javier@osg.samsung.com> - 2015-08-24 09:20 +0200

#1210207 — [PATCH 13/18] iio: adc: mcp320x: Set struct spi_driver .of_match_table

FromJavier Martinez Canillas <javier@osg.samsung.com>
Date2015-08-20 09:10 +0200
Subject[PATCH 13/18] iio: adc: mcp320x: Set struct spi_driver .of_match_table
Message-ID<pZs94-65q-9@gated-at.bofh.it>
The driver has an OF id table but the .of_match_table is not set so
the SPI core can't do an OF style match and the table was unused.

Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
---

 drivers/iio/adc/mcp320x.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/iio/adc/mcp320x.c b/drivers/iio/adc/mcp320x.c
index b19e4f9d16e0..41a21e986c1a 100644
--- a/drivers/iio/adc/mcp320x.c
+++ b/drivers/iio/adc/mcp320x.c
@@ -404,6 +404,7 @@ MODULE_DEVICE_TABLE(spi, mcp320x_id);
 static struct spi_driver mcp320x_driver = {
 	.driver = {
 		.name = "mcp320x",
+		.of_match_table = of_match_ptr(mcp320x_dt_ids),
 		.owner = THIS_MODULE,
 	},
 	.probe = mcp320x_probe,
-- 
2.4.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]


#1210712 — Re: [PATCH 13/18] iio: adc: mcp320x: Set struct spi_driver .of_match_table

FromMichael Welling <mwelling@ieee.org>
Date2015-08-20 22:10 +0200
SubjectRe: [PATCH 13/18] iio: adc: mcp320x: Set struct spi_driver .of_match_table
Message-ID<pZEjT-6Mz-3@gated-at.bofh.it>
In reply to#1210207
On Thu, Aug 20, 2015 at 09:07:26AM +0200, Javier Martinez Canillas wrote:
> The driver has an OF id table but the .of_match_table is not set so
> the SPI core can't do an OF style match and the table was unused.
> 

Is an OF style match necessary?

I have been using devicetree and it matches based on the .id_table.

Couldn't we just remove the mcp320x_dt_ids table instead?

> Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
> ---
> 
>  drivers/iio/adc/mcp320x.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/iio/adc/mcp320x.c b/drivers/iio/adc/mcp320x.c
> index b19e4f9d16e0..41a21e986c1a 100644
> --- a/drivers/iio/adc/mcp320x.c
> +++ b/drivers/iio/adc/mcp320x.c
> @@ -404,6 +404,7 @@ MODULE_DEVICE_TABLE(spi, mcp320x_id);
>  static struct spi_driver mcp320x_driver = {
>  	.driver = {
>  		.name = "mcp320x",
> +		.of_match_table = of_match_ptr(mcp320x_dt_ids),
>  		.owner = THIS_MODULE,
>  	},
>  	.probe = mcp320x_probe,
> -- 
> 2.4.3
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1210756 — Re: [PATCH 13/18] iio: adc: mcp320x: Set struct spi_driver .of_match_table

FromJavier Martinez Canillas <javier@osg.samsung.com>
Date2015-08-21 00:10 +0200
SubjectRe: [PATCH 13/18] iio: adc: mcp320x: Set struct spi_driver .of_match_table
Message-ID<pZGc3-16Z-35@gated-at.bofh.it>
In reply to#1210712
Hello Michael,

On 08/20/2015 10:09 PM, Michael Welling wrote:
> On Thu, Aug 20, 2015 at 09:07:26AM +0200, Javier Martinez Canillas wrote:
>> The driver has an OF id table but the .of_match_table is not set so
>> the SPI core can't do an OF style match and the table was unused.
>>
> 
> Is an OF style match necessary?
>

Did you read the cover letter [0] on which I explain why is needed to
avoid breaking module autoloading in the future? Once the SPI core is
changed by RFC patch 18/18? (you were cc'ed in the cover letter BTW).

> I have been using devicetree and it matches based on the .id_table.
>

Yes it fallbacks to the .id_table or the driver name but the correct
thing to do for devices registered by OF, is to match using the
compatible string.

> Couldn't we just remove the mcp320x_dt_ids table instead?
>

No, that is the wrong thing to do IMHO since the compatible string
contains both vendor and device name whle the .id_table only contains
a device name.

So it makes sense to match using the compatible string and also report
the OF modalias information to user-space.

Otherwise what's the point of the vendor in the compatible string for
SPI devices? You can just use "bar" instead of "foo,bar" as a string.

[0]: https://lkml.org/lkml/2015/8/20/109

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1210765 — Re: [PATCH 13/18] iio: adc: mcp320x: Set struct spi_driver .of_match_table

FromMichael Welling <mwelling@ieee.org>
Date2015-08-21 00:30 +0200
SubjectRe: [PATCH 13/18] iio: adc: mcp320x: Set struct spi_driver .of_match_table
Message-ID<pZGvo-1vS-23@gated-at.bofh.it>
In reply to#1210756
On Fri, Aug 21, 2015 at 12:02:40AM +0200, Javier Martinez Canillas wrote:
> Hello Michael,
> 
> On 08/20/2015 10:09 PM, Michael Welling wrote:
> > On Thu, Aug 20, 2015 at 09:07:26AM +0200, Javier Martinez Canillas wrote:
> >> The driver has an OF id table but the .of_match_table is not set so
> >> the SPI core can't do an OF style match and the table was unused.
> >>
> > 
> > Is an OF style match necessary?
> >
> 
> Did you read the cover letter [0] on which I explain why is needed to
> avoid breaking module autoloading in the future? Once the SPI core is
> changed by RFC patch 18/18? (you were cc'ed in the cover letter BTW).

Well I have read it now. :)

> 
> > I have been using devicetree and it matches based on the .id_table.
> >
> 
> Yes it fallbacks to the .id_table or the driver name but the correct
> thing to do for devices registered by OF, is to match using the
> compatible string.
> 
> > Couldn't we just remove the mcp320x_dt_ids table instead?
> >
> 
> No, that is the wrong thing to do IMHO since the compatible string
> contains both vendor and device name whle the .id_table only contains
> a device name.
> 
> So it makes sense to match using the compatible string and also report
> the OF modalias information to user-space.
> 
> Otherwise what's the point of the vendor in the compatible string for
> SPI devices? You can just use "bar" instead of "foo,bar" as a string.
> 

Well then shouldn't the patch include adding the vendor to the compatible
string?


> [0]: https://lkml.org/lkml/2015/8/20/109
> 
> Best regards,
> -- 
> Javier Martinez Canillas
> Open Source Group
> Samsung Research America
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1210771 — Re: [PATCH 13/18] iio: adc: mcp320x: Set struct spi_driver .of_match_table

FromJavier Martinez Canillas <javier@osg.samsung.com>
Date2015-08-21 00:50 +0200
SubjectRe: [PATCH 13/18] iio: adc: mcp320x: Set struct spi_driver .of_match_table
Message-ID<pZGOK-1Ug-29@gated-at.bofh.it>
In reply to#1210765
Hello Michael,

On 08/21/2015 12:29 AM, Michael Welling wrote:
> On Fri, Aug 21, 2015 at 12:02:40AM +0200, Javier Martinez Canillas wrote:
>> Hello Michael,
>>
>> On 08/20/2015 10:09 PM, Michael Welling wrote:
>>> On Thu, Aug 20, 2015 at 09:07:26AM +0200, Javier Martinez Canillas wrote:
>>>> The driver has an OF id table but the .of_match_table is not set so
>>>> the SPI core can't do an OF style match and the table was unused.
>>>>
>>>
>>> Is an OF style match necessary?
>>>
>>
>> Did you read the cover letter [0] on which I explain why is needed to
>> avoid breaking module autoloading in the future? Once the SPI core is
>> changed by RFC patch 18/18? (you were cc'ed in the cover letter BTW).
> 
> Well I have read it now. :)
>

Great :)
 
>>
>>> I have been using devicetree and it matches based on the .id_table.
>>>
>>
>> Yes it fallbacks to the .id_table or the driver name but the correct
>> thing to do for devices registered by OF, is to match using the
>> compatible string.
>>
>>> Couldn't we just remove the mcp320x_dt_ids table instead?
>>>
>>
>> No, that is the wrong thing to do IMHO since the compatible string
>> contains both vendor and device name whle the .id_table only contains
>> a device name.
>>
>> So it makes sense to match using the compatible string and also report
>> the OF modalias information to user-space.
>>
>> Otherwise what's the point of the vendor in the compatible string for
>> SPI devices? You can just use "bar" instead of "foo,bar" as a string.
>>
> 
> Well then shouldn't the patch include adding the vendor to the compatible
> string?
>

Well, I was talking in general. You are right that this specific driver does
not have a vendor prefix for the compatible strings. This is incorrect
according to the ePAPR document [0].

However, these compatible strings are already documented as a DT binding doc
in Documentation/devicetree/bindings/iio/adc/mcp320x.txt so I don't know
what is the correct thing to do in this situation.

Changing the compatible string will break old DTB with newer kernels and that
is a no go. But that doesn't invalidate what I said since once the SPI core
is changed and report OF modalias, you will need a MODULE_DEVICE_TABLE(of,...)
regardless if the compatible string has a vendor prefix or not.

[0]: https://www.power.org/documentation/epapr-version-1-1/

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1210799 — Re: [PATCH 13/18] iio: adc: mcp320x: Set struct spi_driver .of_match_table

FromMichael Welling <mwelling@ieee.org>
Date2015-08-21 02:00 +0200
SubjectRe: [PATCH 13/18] iio: adc: mcp320x: Set struct spi_driver .of_match_table
Message-ID<pZHUt-3qg-5@gated-at.bofh.it>
In reply to#1210771
On Fri, Aug 21, 2015 at 12:48:23AM +0200, Javier Martinez Canillas wrote:
> Hello Michael,
> 
> On 08/21/2015 12:29 AM, Michael Welling wrote:
> > On Fri, Aug 21, 2015 at 12:02:40AM +0200, Javier Martinez Canillas wrote:
> >> Hello Michael,
> >>
> >> On 08/20/2015 10:09 PM, Michael Welling wrote:
> >>> On Thu, Aug 20, 2015 at 09:07:26AM +0200, Javier Martinez Canillas wrote:
> >>>> The driver has an OF id table but the .of_match_table is not set so
> >>>> the SPI core can't do an OF style match and the table was unused.
> >>>>
> >>>
> >>> Is an OF style match necessary?
> >>>
> >>
> >> Did you read the cover letter [0] on which I explain why is needed to
> >> avoid breaking module autoloading in the future? Once the SPI core is
> >> changed by RFC patch 18/18? (you were cc'ed in the cover letter BTW).
> > 
> > Well I have read it now. :)
> >
> 
> Great :)
>  
> >>
> >>> I have been using devicetree and it matches based on the .id_table.
> >>>
> >>
> >> Yes it fallbacks to the .id_table or the driver name but the correct
> >> thing to do for devices registered by OF, is to match using the
> >> compatible string.
> >>
> >>> Couldn't we just remove the mcp320x_dt_ids table instead?
> >>>
> >>
> >> No, that is the wrong thing to do IMHO since the compatible string
> >> contains both vendor and device name whle the .id_table only contains
> >> a device name.
> >>
> >> So it makes sense to match using the compatible string and also report
> >> the OF modalias information to user-space.
> >>
> >> Otherwise what's the point of the vendor in the compatible string for
> >> SPI devices? You can just use "bar" instead of "foo,bar" as a string.
> >>
> > 
> > Well then shouldn't the patch include adding the vendor to the compatible
> > string?
> >
> 
> Well, I was talking in general. You are right that this specific driver does
> not have a vendor prefix for the compatible strings. This is incorrect
> according to the ePAPR document [0].
> 
> However, these compatible strings are already documented as a DT binding doc
> in Documentation/devicetree/bindings/iio/adc/mcp320x.txt so I don't know
> what is the correct thing to do in this situation.
> 
> Changing the compatible string will break old DTB with newer kernels and that
> is a no go. But that doesn't invalidate what I said since once the SPI core
> is changed and report OF modalias, you will need a MODULE_DEVICE_TABLE(of,...)
> regardless if the compatible string has a vendor prefix or not.
>

Okay.

Acked-by: Michael Welling <mwelling@ieee.org>
 
> [0]: https://www.power.org/documentation/epapr-version-1-1/
> 
> Best regards,
> -- 
> Javier Martinez Canillas
> Open Source Group
> Samsung Research America
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1211489 — Re: [PATCH 13/18] iio: adc: mcp320x: Set struct spi_driver .of_match_table

FromJonathan Cameron <jic23@kernel.org>
Date2015-08-22 20:10 +0200
SubjectRe: [PATCH 13/18] iio: adc: mcp320x: Set struct spi_driver .of_match_table
Message-ID<q0loR-1f3-1@gated-at.bofh.it>
In reply to#1210799
On 21/08/15 00:53, Michael Welling wrote:
> On Fri, Aug 21, 2015 at 12:48:23AM +0200, Javier Martinez Canillas wrote:
>> Hello Michael,
>>
>> On 08/21/2015 12:29 AM, Michael Welling wrote:
>>> On Fri, Aug 21, 2015 at 12:02:40AM +0200, Javier Martinez Canillas wrote:
>>>> Hello Michael,
>>>>
>>>> On 08/20/2015 10:09 PM, Michael Welling wrote:
>>>>> On Thu, Aug 20, 2015 at 09:07:26AM +0200, Javier Martinez Canillas wrote:
>>>>>> The driver has an OF id table but the .of_match_table is not set so
>>>>>> the SPI core can't do an OF style match and the table was unused.
>>>>>>
>>>>>
>>>>> Is an OF style match necessary?
>>>>>
>>>>
>>>> Did you read the cover letter [0] on which I explain why is needed to
>>>> avoid breaking module autoloading in the future? Once the SPI core is
>>>> changed by RFC patch 18/18? (you were cc'ed in the cover letter BTW).
>>>
>>> Well I have read it now. :)
>>>
>>
>> Great :)
>>  
>>>>
>>>>> I have been using devicetree and it matches based on the .id_table.
>>>>>
>>>>
>>>> Yes it fallbacks to the .id_table or the driver name but the correct
>>>> thing to do for devices registered by OF, is to match using the
>>>> compatible string.
>>>>
>>>>> Couldn't we just remove the mcp320x_dt_ids table instead?
>>>>>
>>>>
>>>> No, that is the wrong thing to do IMHO since the compatible string
>>>> contains both vendor and device name whle the .id_table only contains
>>>> a device name.
>>>>
>>>> So it makes sense to match using the compatible string and also report
>>>> the OF modalias information to user-space.
>>>>
>>>> Otherwise what's the point of the vendor in the compatible string for
>>>> SPI devices? You can just use "bar" instead of "foo,bar" as a string.
>>>>
>>>
>>> Well then shouldn't the patch include adding the vendor to the compatible
>>> string?
>>>
>>
>> Well, I was talking in general. You are right that this specific driver does
>> not have a vendor prefix for the compatible strings. This is incorrect
>> according to the ePAPR document [0].
>>
>> However, these compatible strings are already documented as a DT binding doc
>> in Documentation/devicetree/bindings/iio/adc/mcp320x.txt so I don't know
>> what is the correct thing to do in this situation.
>>
>> Changing the compatible string will break old DTB with newer kernels and that
>> is a no go. But that doesn't invalidate what I said since once the SPI core
>> is changed and report OF modalias, you will need a MODULE_DEVICE_TABLE(of,...)
>> regardless if the compatible string has a vendor prefix or not.
>>
> 
> Okay.
> 
> Acked-by: Michael Welling <mwelling@ieee.org>
Applied to the togreg branch of iio.git

Thanks,

Jonathan
>  
>> [0]: https://www.power.org/documentation/epapr-version-1-1/
>>
>> Best regards,
>> -- 
>> Javier Martinez Canillas
>> Open Source Group
>> Samsung Research America
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1211486 — Re: [PATCH 13/18] iio: adc: mcp320x: Set struct spi_driver .of_match_table

FromJonathan Cameron <jic23@kernel.org>
Date2015-08-22 19:50 +0200
SubjectRe: [PATCH 13/18] iio: adc: mcp320x: Set struct spi_driver .of_match_table
Message-ID<q0l5w-D7-1@gated-at.bofh.it>
In reply to#1210771
On 20/08/15 23:48, Javier Martinez Canillas wrote:
> Hello Michael,
> 
> On 08/21/2015 12:29 AM, Michael Welling wrote:
>> On Fri, Aug 21, 2015 at 12:02:40AM +0200, Javier Martinez Canillas wrote:
>>> Hello Michael,
>>>
>>> On 08/20/2015 10:09 PM, Michael Welling wrote:
>>>> On Thu, Aug 20, 2015 at 09:07:26AM +0200, Javier Martinez Canillas wrote:
>>>>> The driver has an OF id table but the .of_match_table is not set so
>>>>> the SPI core can't do an OF style match and the table was unused.
>>>>>
>>>>
>>>> Is an OF style match necessary?
>>>>
>>>
>>> Did you read the cover letter [0] on which I explain why is needed to
>>> avoid breaking module autoloading in the future? Once the SPI core is
>>> changed by RFC patch 18/18? (you were cc'ed in the cover letter BTW).
>>
>> Well I have read it now. :)
>>
> 
> Great :)
>  
>>>
>>>> I have been using devicetree and it matches based on the .id_table.
>>>>
>>>
>>> Yes it fallbacks to the .id_table or the driver name but the correct
>>> thing to do for devices registered by OF, is to match using the
>>> compatible string.
>>>
>>>> Couldn't we just remove the mcp320x_dt_ids table instead?
>>>>
>>>
>>> No, that is the wrong thing to do IMHO since the compatible string
>>> contains both vendor and device name whle the .id_table only contains
>>> a device name.
>>>
>>> So it makes sense to match using the compatible string and also report
>>> the OF modalias information to user-space.
>>>
>>> Otherwise what's the point of the vendor in the compatible string for
>>> SPI devices? You can just use "bar" instead of "foo,bar" as a string.
>>>
>>
>> Well then shouldn't the patch include adding the vendor to the compatible
>> string?
>>
> 
> Well, I was talking in general. You are right that this specific driver does
> not have a vendor prefix for the compatible strings. This is incorrect
> according to the ePAPR document [0].
> 
> However, these compatible strings are already documented as a DT binding doc
> in Documentation/devicetree/bindings/iio/adc/mcp320x.txt so I don't know
> what is the correct thing to do in this situation.
Take the view the old version is wrong but needs to be supported and add also
the corrected strings + document them.

cc'd The device tree list for any more comments on this.
> 
> Changing the compatible string will break old DTB with newer kernels and that
> is a no go. But that doesn't invalidate what I said since once the SPI core
> is changed and report OF modalias, you will need a MODULE_DEVICE_TABLE(of,...)
> regardless if the compatible string has a vendor prefix or not.
> 
> [0]: https://www.power.org/documentation/epapr-version-1-1/
> 
> Best regards,
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1211675

FromRob Herring <robherring2@gmail.com>
Date2015-08-24 00:20 +0200
Message-ID<q0LMm-5vW-13@gated-at.bofh.it>
In reply to#1211486
On Sat, Aug 22, 2015 at 12:48 PM, Jonathan Cameron <jic23@kernel.org> wrote:
> On 20/08/15 23:48, Javier Martinez Canillas wrote:
>> Hello Michael,
>>
>> On 08/21/2015 12:29 AM, Michael Welling wrote:
>>> On Fri, Aug 21, 2015 at 12:02:40AM +0200, Javier Martinez Canillas wrote:
>>>> Hello Michael,
>>>>
>>>> On 08/20/2015 10:09 PM, Michael Welling wrote:
>>>>> On Thu, Aug 20, 2015 at 09:07:26AM +0200, Javier Martinez Canillas wrote:
>>>>>> The driver has an OF id table but the .of_match_table is not set so
>>>>>> the SPI core can't do an OF style match and the table was unused.
>>>>>>
>>>>>
>>>>> Is an OF style match necessary?
>>>>>
>>>>
>>>> Did you read the cover letter [0] on which I explain why is needed to
>>>> avoid breaking module autoloading in the future? Once the SPI core is
>>>> changed by RFC patch 18/18? (you were cc'ed in the cover letter BTW).
>>>
>>> Well I have read it now. :)
>>>
>>
>> Great :)
>>
>>>>
>>>>> I have been using devicetree and it matches based on the .id_table.
>>>>>
>>>>
>>>> Yes it fallbacks to the .id_table or the driver name but the correct
>>>> thing to do for devices registered by OF, is to match using the
>>>> compatible string.
>>>>
>>>>> Couldn't we just remove the mcp320x_dt_ids table instead?
>>>>>
>>>>
>>>> No, that is the wrong thing to do IMHO since the compatible string
>>>> contains both vendor and device name whle the .id_table only contains
>>>> a device name.
>>>>
>>>> So it makes sense to match using the compatible string and also report
>>>> the OF modalias information to user-space.
>>>>
>>>> Otherwise what's the point of the vendor in the compatible string for
>>>> SPI devices? You can just use "bar" instead of "foo,bar" as a string.
>>>>
>>>
>>> Well then shouldn't the patch include adding the vendor to the compatible
>>> string?
>>>
>>
>> Well, I was talking in general. You are right that this specific driver does
>> not have a vendor prefix for the compatible strings. This is incorrect
>> according to the ePAPR document [0].
>>
>> However, these compatible strings are already documented as a DT binding doc
>> in Documentation/devicetree/bindings/iio/adc/mcp320x.txt so I don't know
>> what is the correct thing to do in this situation.
> Take the view the old version is wrong but needs to be supported and add also
> the corrected strings + document them.
>
> cc'd The device tree list for any more comments on this.

Agreed. Document both and mark the old one deprecated.

Rob
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1211848 — Re: [PATCH 13/18] iio: adc: mcp320x: Set struct spi_driver .of_match_table

FromJavier Martinez Canillas <javier@osg.samsung.com>
Date2015-08-24 09:20 +0200
SubjectRe: [PATCH 13/18] iio: adc: mcp320x: Set struct spi_driver .of_match_table
Message-ID<q0UcW-Hu-29@gated-at.bofh.it>
In reply to#1211675
Hello Rob and Jonathan,

On 08/24/2015 12:10 AM, Rob Herring wrote:
> On Sat, Aug 22, 2015 at 12:48 PM, Jonathan Cameron <jic23@kernel.org> wrote:
>> On 20/08/15 23:48, Javier Martinez Canillas wrote:
>>> Hello Michael,
>>>
>>> On 08/21/2015 12:29 AM, Michael Welling wrote:
>>>> On Fri, Aug 21, 2015 at 12:02:40AM +0200, Javier Martinez Canillas wrote:
>>>>> Hello Michael,
>>>>>
>>>>> On 08/20/2015 10:09 PM, Michael Welling wrote:
>>>>>> On Thu, Aug 20, 2015 at 09:07:26AM +0200, Javier Martinez Canillas wrote:
>>>>>>> The driver has an OF id table but the .of_match_table is not set so
>>>>>>> the SPI core can't do an OF style match and the table was unused.
>>>>>>>
>>>>>>
>>>>>> Is an OF style match necessary?
>>>>>>
>>>>>
>>>>> Did you read the cover letter [0] on which I explain why is needed to
>>>>> avoid breaking module autoloading in the future? Once the SPI core is
>>>>> changed by RFC patch 18/18? (you were cc'ed in the cover letter BTW).
>>>>
>>>> Well I have read it now. :)
>>>>
>>>
>>> Great :)
>>>
>>>>>
>>>>>> I have been using devicetree and it matches based on the .id_table.
>>>>>>
>>>>>
>>>>> Yes it fallbacks to the .id_table or the driver name but the correct
>>>>> thing to do for devices registered by OF, is to match using the
>>>>> compatible string.
>>>>>
>>>>>> Couldn't we just remove the mcp320x_dt_ids table instead?
>>>>>>
>>>>>
>>>>> No, that is the wrong thing to do IMHO since the compatible string
>>>>> contains both vendor and device name whle the .id_table only contains
>>>>> a device name.
>>>>>
>>>>> So it makes sense to match using the compatible string and also report
>>>>> the OF modalias information to user-space.
>>>>>
>>>>> Otherwise what's the point of the vendor in the compatible string for
>>>>> SPI devices? You can just use "bar" instead of "foo,bar" as a string.
>>>>>
>>>>
>>>> Well then shouldn't the patch include adding the vendor to the compatible
>>>> string?
>>>>
>>>
>>> Well, I was talking in general. You are right that this specific driver does
>>> not have a vendor prefix for the compatible strings. This is incorrect
>>> according to the ePAPR document [0].
>>>
>>> However, these compatible strings are already documented as a DT binding doc
>>> in Documentation/devicetree/bindings/iio/adc/mcp320x.txt so I don't know
>>> what is the correct thing to do in this situation.
>> Take the view the old version is wrong but needs to be supported and add also
>> the corrected strings + document them.
>>
>> cc'd The device tree list for any more comments on this.
> 
> Agreed. Document both and mark the old one deprecated.
>

Thanks a lot for your comments. I'll do that as a follow up then.
 
> Rob
> 

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web