Re: [PATCH 13/18] iio: adc: mcp320x: Set struct spi_driver .of_match_table
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 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 13/18] iio: adc: mcp320x: Set struct spi_driver .of_match_table
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 ji...@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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 13/18] iio: adc: mcp320x: Set struct spi_driver .of_match_table
On Sat, Aug 22, 2015 at 12:48 PM, Jonathan Cameron 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 13/18] iio: adc: mcp320x: Set struct spi_driver .of_match_table
On Sat, Aug 22, 2015 at 12:48 PM, Jonathan Cameron ji...@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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 13/18] iio: adc: mcp320x: Set struct spi_driver .of_match_table
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 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 majord...@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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 13/18] iio: adc: mcp320x: Set struct spi_driver .of_match_table
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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 13/18] iio: adc: mcp320x: Set struct spi_driver .of_match_table
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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 13/18] iio: adc: mcp320x: Set struct spi_driver .of_match_table
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 mwell...@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 majord...@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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 13/18] iio: adc: mcp320x: Set struct spi_driver .of_match_table
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 > [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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 13/18] iio: adc: mcp320x: Set struct spi_driver .of_match_table
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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 13/18] iio: adc: mcp320x: Set struct spi_driver .of_match_table
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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 13/18] iio: adc: mcp320x: Set struct spi_driver .of_match_table
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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 13/18] iio: adc: mcp320x: Set struct spi_driver .of_match_table
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
> ---
>
> 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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 13/18] iio: adc: mcp320x: Set struct spi_driver .of_match_table
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
---
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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 13/18] iio: adc: mcp320x: Set struct spi_driver .of_match_table
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 jav...@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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 13/18] iio: adc: mcp320x: Set struct spi_driver .of_match_table
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 mwell...@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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 13/18] iio: adc: mcp320x: Set struct spi_driver .of_match_table
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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 13/18] iio: adc: mcp320x: Set struct spi_driver .of_match_table
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 jav...@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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 13/18] iio: adc: mcp320x: Set struct spi_driver .of_match_table
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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 13/18] iio: adc: mcp320x: Set struct spi_driver .of_match_table
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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
