| Message ID | c79bdd79a22dab877b8a80136ac253c951698745.1482936802.git.hns@goldelico.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Dec 28, 2016 at 03:53:21PM +0100, H. Nikolaus Schaller wrote: > Fix module table so that the driver is loaded if compiled > as module and requested by DT. > I believe I already replied to a similar patch: we alreadyhave necessary aliases in this driver, we need to fix module loading to use it. Thanks.
Hi Dmitry, > Am 28.01.2017 um 20:35 schrieb Dmitry Torokhov <dmitry.torokhov@gmail.com>: > > On Wed, Dec 28, 2016 at 03:53:21PM +0100, H. Nikolaus Schaller wrote: >> Fix module table so that the driver is loaded if compiled >> as module and requested by DT. >> > > I believe I already replied to a similar patch: we alreadyhave necessary > aliases in this driver, we need to fix module loading to use it. Yes, you did comment on [PATCH v6 7/8] (19 Nov 2016): >> We really need to fix it between spi/i23c core and module utils instead >> of keeping adding duplicate IDs all over drivers. We already have OF >> module device table containing the same data, we should be able to use >> it. And Javier Martinez Canillas replied (23 Nov 2016): > Agreed, unfortunately until the I2C and SPI core are changed to properly > report OF modaliases, we will have to keep adding these duplicated IDs. > > And changing the I2C and SPI core isn't trivial since it could break a > lot of drivers that rely on a platform modalias being reported (i.e: no > OF device IDs present in the drivers even when are registered via DT). Therefore I didn't see a need to change it. BR, Nikolaus -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sun, Jan 29, 2017 at 09:39:39AM +0100, H. Nikolaus Schaller wrote: > Hi Dmitry, > > > Am 28.01.2017 um 20:35 schrieb Dmitry Torokhov <dmitry.torokhov@gmail.com>: > > > > On Wed, Dec 28, 2016 at 03:53:21PM +0100, H. Nikolaus Schaller wrote: > >> Fix module table so that the driver is loaded if compiled > >> as module and requested by DT. > >> > > > > I believe I already replied to a similar patch: we alreadyhave necessary > > aliases in this driver, we need to fix module loading to use it. > > Yes, you did comment on [PATCH v6 7/8] (19 Nov 2016): > > >> We really need to fix it between spi/i23c core and module utils instead > >> of keeping adding duplicate IDs all over drivers. We already have OF > >> module device table containing the same data, we should be able to use > >> it. > > And Javier Martinez Canillas replied (23 Nov 2016): > > > Agreed, unfortunately until the I2C and SPI core are changed to properly > > report OF modaliases, we will have to keep adding these duplicated IDs. > > > > And changing the I2C and SPI core isn't trivial since it could break a > > lot of drivers that rely on a platform modalias being reported (i.e: no > > OF device IDs present in the drivers even when are registered via DT). > > Therefore I didn't see a need to change it. I agree that changing I2C and SPI core is not trivial, however this is no reason for piling up workarounds in all drivers. Are you seriously advocating going though *every* driver and copying OF data into I2C/SPI instead of doing the right thing and fixing the root of the issue? Thanks.
Hi Dmitry, > Am 29.01.2017 um 19:01 schrieb Dmitry Torokhov <dmitry.torokhov@gmail.com>: > > On Sun, Jan 29, 2017 at 09:39:39AM +0100, H. Nikolaus Schaller wrote: >> Hi Dmitry, >> >>> Am 28.01.2017 um 20:35 schrieb Dmitry Torokhov <dmitry.torokhov@gmail.com>: >>> >>> On Wed, Dec 28, 2016 at 03:53:21PM +0100, H. Nikolaus Schaller wrote: >>>> Fix module table so that the driver is loaded if compiled >>>> as module and requested by DT. >>>> >>> >>> I believe I already replied to a similar patch: we alreadyhave necessary >>> aliases in this driver, we need to fix module loading to use it. >> >> Yes, you did comment on [PATCH v6 7/8] (19 Nov 2016): >> >>>> We really need to fix it between spi/i23c core and module utils instead >>>> of keeping adding duplicate IDs all over drivers. We already have OF >>>> module device table containing the same data, we should be able to use >>>> it. >> >> And Javier Martinez Canillas replied (23 Nov 2016): >> >>> Agreed, unfortunately until the I2C and SPI core are changed to properly >>> report OF modaliases, we will have to keep adding these duplicated IDs. >>> >>> And changing the I2C and SPI core isn't trivial since it could break a >>> lot of drivers that rely on a platform modalias being reported (i.e: no >>> OF device IDs present in the drivers even when are registered via DT). >> >> Therefore I didn't see a need to change it. > > I agree that changing I2C and SPI core is not trivial, however this is > no reason for piling up workarounds in all drivers. Are you seriously > advocating going though *every* driver and copying OF data into I2C/SPI > instead of doing the right thing and fixing the root of the issue? If you prefer to have this done (and I agree it would be a tiny improvement), please do it for us all. But please after merging this workaround. I can't do it since I have no idea how to and as you say it is non-trivial. I am just a poor driver developer who has to use what he finds in core APIs but has the task to make the driver work and be accepted upstream. What I don't like is if core development tasks are piggybacked on top of a driver patch series and used as argument against acceptance of the driver patch. IMHO, the core developers should have the task to clean up such workarounds after developing a modification for core and it seems to be common practise in other subsystems if you follow what Linus is merging every week. Anyways, it piles up only in code. If you do not CONFIG the driver it does not even matter that there are 10 more source code lines. And they are factored out into a clean patch 6/8 that can easily be reverted. BR and thanks, Nikolaus -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Dmitry, Javier, > Am 29.01.2017 um 19:25 schrieb H. Nikolaus Schaller <hns@goldelico.com>: > > Hi Dmitry, > >> Am 29.01.2017 um 19:01 schrieb Dmitry Torokhov <dmitry.torokhov@gmail.com>: >> >> On Sun, Jan 29, 2017 at 09:39:39AM +0100, H. Nikolaus Schaller wrote: >>> Hi Dmitry, >>> >>>> Am 28.01.2017 um 20:35 schrieb Dmitry Torokhov <dmitry.torokhov@gmail.com>: >>>> >>>> On Wed, Dec 28, 2016 at 03:53:21PM +0100, H. Nikolaus Schaller wrote: >>>>> Fix module table so that the driver is loaded if compiled >>>>> as module and requested by DT. >>>>> >>>> >>>> I believe I already replied to a similar patch: we alreadyhave necessary >>>> aliases in this driver, we need to fix module loading to use it. >>> >>> Yes, you did comment on [PATCH v6 7/8] (19 Nov 2016): >>> >>>>> We really need to fix it between spi/i23c core and module utils instead >>>>> of keeping adding duplicate IDs all over drivers. We already have OF >>>>> module device table containing the same data, we should be able to use >>>>> it. >>> >>> And Javier Martinez Canillas replied (23 Nov 2016): >>> >>>> Agreed, unfortunately until the I2C and SPI core are changed to properly >>>> report OF modaliases, we will have to keep adding these duplicated IDs. >>>> >>>> And changing the I2C and SPI core isn't trivial since it could break a >>>> lot of drivers that rely on a platform modalias being reported (i.e: no >>>> OF device IDs present in the drivers even when are registered via DT). >>> >>> Therefore I didn't see a need to change it. >> >> I agree that changing I2C and SPI core is not trivial, however this is >> no reason for piling up workarounds in all drivers. Are you seriously >> advocating going though *every* driver and copying OF data into I2C/SPI >> instead of doing the right thing and fixing the root of the issue? > > If you prefer to have this done (and I agree it would be a tiny improvement), > please do it for us all. But please after merging this workaround. Have we been lucky to find someone who is able and willing to work on this? If not, I'd recommend to stay with the current level of optimality. BR and thanks, Nikolaus -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello Nikolaus, On 02/01/2017 05:20 PM, H. Nikolaus Schaller wrote: > Hi Dmitry, Javier, > >> Am 29.01.2017 um 19:25 schrieb H. Nikolaus Schaller <hns@goldelico.com>: >> >> Hi Dmitry, >> >>> Am 29.01.2017 um 19:01 schrieb Dmitry Torokhov <dmitry.torokhov@gmail.com>: >>> >>> On Sun, Jan 29, 2017 at 09:39:39AM +0100, H. Nikolaus Schaller wrote: >>>> Hi Dmitry, >>>> >>>>> Am 28.01.2017 um 20:35 schrieb Dmitry Torokhov <dmitry.torokhov@gmail.com>: >>>>> >>>>> On Wed, Dec 28, 2016 at 03:53:21PM +0100, H. Nikolaus Schaller wrote: >>>>>> Fix module table so that the driver is loaded if compiled >>>>>> as module and requested by DT. >>>>>> >>>>> >>>>> I believe I already replied to a similar patch: we alreadyhave necessary >>>>> aliases in this driver, we need to fix module loading to use it. >>>> >>>> Yes, you did comment on [PATCH v6 7/8] (19 Nov 2016): >>>> >>>>>> We really need to fix it between spi/i23c core and module utils instead >>>>>> of keeping adding duplicate IDs all over drivers. We already have OF >>>>>> module device table containing the same data, we should be able to use >>>>>> it. >>>> >>>> And Javier Martinez Canillas replied (23 Nov 2016): >>>> >>>>> Agreed, unfortunately until the I2C and SPI core are changed to properly >>>>> report OF modaliases, we will have to keep adding these duplicated IDs. >>>>> >>>>> And changing the I2C and SPI core isn't trivial since it could break a >>>>> lot of drivers that rely on a platform modalias being reported (i.e: no >>>>> OF device IDs present in the drivers even when are registered via DT). >>>> >>>> Therefore I didn't see a need to change it. >>> >>> I agree that changing I2C and SPI core is not trivial, however this is >>> no reason for piling up workarounds in all drivers. Are you seriously >>> advocating going though *every* driver and copying OF data into I2C/SPI >>> instead of doing the right thing and fixing the root of the issue? >> >> If you prefer to have this done (and I agree it would be a tiny improvement), >> please do it for us all. But please after merging this workaround. > > Have we been lucky to find someone who is able and willing to work on this? > As said, changing the core is trivial. A RFC patch is [0]. The problem is how to make sure that this change won't cause regressions in existing drivers. There was particularly tricky for the spi-nor driver, it doesn't help that a lot of DT are using undocumented compatible strings (sometimes with no vendor prefix). You can see the discussion here [1]. In the same thread Mark Brown said that it wasn't that bad to have the information in the OF device ID table duplicated in a SPI device table. Certainly isn't the best approach but IMHO is better than the module not loading. https://patchwork.kernel.org/patch/7041141/ https://patchwork.ozlabs.org/patch/545125/ > If not, I'd recommend to stay with the current level of optimality. > > BR and thanks, > Nikolaus > Best regards,
On Wed, Feb 01, 2017 at 06:14:39PM -0300, Javier Martinez Canillas wrote: > Hello Nikolaus, > > On 02/01/2017 05:20 PM, H. Nikolaus Schaller wrote: > > Hi Dmitry, Javier, > > > >> Am 29.01.2017 um 19:25 schrieb H. Nikolaus Schaller <hns@goldelico.com>: > >> > >> Hi Dmitry, > >> > >>> Am 29.01.2017 um 19:01 schrieb Dmitry Torokhov <dmitry.torokhov@gmail.com>: > >>> > >>> On Sun, Jan 29, 2017 at 09:39:39AM +0100, H. Nikolaus Schaller wrote: > >>>> Hi Dmitry, > >>>> > >>>>> Am 28.01.2017 um 20:35 schrieb Dmitry Torokhov <dmitry.torokhov@gmail.com>: > >>>>> > >>>>> On Wed, Dec 28, 2016 at 03:53:21PM +0100, H. Nikolaus Schaller wrote: > >>>>>> Fix module table so that the driver is loaded if compiled > >>>>>> as module and requested by DT. > >>>>>> > >>>>> > >>>>> I believe I already replied to a similar patch: we alreadyhave necessary > >>>>> aliases in this driver, we need to fix module loading to use it. > >>>> > >>>> Yes, you did comment on [PATCH v6 7/8] (19 Nov 2016): > >>>> > >>>>>> We really need to fix it between spi/i23c core and module utils instead > >>>>>> of keeping adding duplicate IDs all over drivers. We already have OF > >>>>>> module device table containing the same data, we should be able to use > >>>>>> it. > >>>> > >>>> And Javier Martinez Canillas replied (23 Nov 2016): > >>>> > >>>>> Agreed, unfortunately until the I2C and SPI core are changed to properly > >>>>> report OF modaliases, we will have to keep adding these duplicated IDs. > >>>>> > >>>>> And changing the I2C and SPI core isn't trivial since it could break a > >>>>> lot of drivers that rely on a platform modalias being reported (i.e: no > >>>>> OF device IDs present in the drivers even when are registered via DT). > >>>> > >>>> Therefore I didn't see a need to change it. > >>> > >>> I agree that changing I2C and SPI core is not trivial, however this is > >>> no reason for piling up workarounds in all drivers. Are you seriously > >>> advocating going though *every* driver and copying OF data into I2C/SPI > >>> instead of doing the right thing and fixing the root of the issue? > >> > >> If you prefer to have this done (and I agree it would be a tiny improvement), > >> please do it for us all. But please after merging this workaround. > > > > Have we been lucky to find someone who is able and willing to work on this? > > > > As said, changing the core is trivial. A RFC patch is [0]. > > The problem is how to make sure that this change won't cause regressions > in existing drivers. If the concern with drivers having I2C or SPI device table, but not OF device table, then I think cocinnelle could be used to scan and find them. > > There was particularly tricky for the spi-nor driver, it doesn't help that > a lot of DT are using undocumented compatible strings (sometimes with no > vendor prefix). You can see the discussion here [1]. > > In the same thread Mark Brown said that it wasn't that bad to have the > information in the OF device ID table duplicated in a SPI device table. > > Certainly isn't the best approach but IMHO is better than the module not > loading. You can always build the module into kernel or load it by hand if it is that important. Module autoloading does not work anyway if you have several compatible strings, like compatible = "vendor,new-device", "vendor,generic-device"; Thanks.
Hello Dmitry, On 02/01/2017 07:28 PM, Dmitry Torokhov wrote: [snip] >> >> As said, changing the core is trivial. A RFC patch is [0]. >> >> The problem is how to make sure that this change won't cause regressions >> in existing drivers. > > If the concern with drivers having I2C or SPI device table, but not OF > device table, then I think cocinnelle could be used to scan and find > them. > I don't think coccinelle is a good fit for this since you may have to look if a DTS is using in its compatible string a device that's in an SPI table or if a DT binding documents this device as a compatible string. But yes I get your point and I agree that is just a matter of having a script to look for these. Someone should have the time to write it though :) >> >> There was particularly tricky for the spi-nor driver, it doesn't help that >> a lot of DT are using undocumented compatible strings (sometimes with no >> vendor prefix). You can see the discussion here [1]. >> >> In the same thread Mark Brown said that it wasn't that bad to have the >> information in the OF device ID table duplicated in a SPI device table. >> >> Certainly isn't the best approach but IMHO is better than the module not >> loading. > > You can always build the module into kernel or load it by hand if it is > that important. Module autoloading does not work anyway if you have > several compatible strings, like > > compatible = "vendor,new-device", "vendor,generic-device"; > > Thanks. > Agree. Best regards,
Hi, > Am 01.02.2017 um 23:28 schrieb Dmitry Torokhov <dmitry.torokhov@gmail.com>: > > On Wed, Feb 01, 2017 at 06:14:39PM -0300, Javier Martinez Canillas wrote: >> Hello Nikolaus, >> >> On 02/01/2017 05:20 PM, H. Nikolaus Schaller wrote: >>> Hi Dmitry, Javier, >>> >>>> Am 29.01.2017 um 19:25 schrieb H. Nikolaus Schaller <hns@goldelico.com>: >>>> >>>> Hi Dmitry, >>>> >>>>> Am 29.01.2017 um 19:01 schrieb Dmitry Torokhov <dmitry.torokhov@gmail.com>: >>>>> >>>>> On Sun, Jan 29, 2017 at 09:39:39AM +0100, H. Nikolaus Schaller wrote: >>>>>> Hi Dmitry, >>>>>> >>>>>>> Am 28.01.2017 um 20:35 schrieb Dmitry Torokhov <dmitry.torokhov@gmail.com>: >>>>>>> >>>>>>> On Wed, Dec 28, 2016 at 03:53:21PM +0100, H. Nikolaus Schaller wrote: >>>>>>>> Fix module table so that the driver is loaded if compiled >>>>>>>> as module and requested by DT. >>>>>>>> >>>>>>> >>>>>>> I believe I already replied to a similar patch: we alreadyhave necessary >>>>>>> aliases in this driver, we need to fix module loading to use it. >>>>>> >>>>>> Yes, you did comment on [PATCH v6 7/8] (19 Nov 2016): >>>>>> >>>>>>>> We really need to fix it between spi/i23c core and module utils instead >>>>>>>> of keeping adding duplicate IDs all over drivers. We already have OF >>>>>>>> module device table containing the same data, we should be able to use >>>>>>>> it. >>>>>> >>>>>> And Javier Martinez Canillas replied (23 Nov 2016): >>>>>> >>>>>>> Agreed, unfortunately until the I2C and SPI core are changed to properly >>>>>>> report OF modaliases, we will have to keep adding these duplicated IDs. >>>>>>> >>>>>>> And changing the I2C and SPI core isn't trivial since it could break a >>>>>>> lot of drivers that rely on a platform modalias being reported (i.e: no >>>>>>> OF device IDs present in the drivers even when are registered via DT). >>>>>> >>>>>> Therefore I didn't see a need to change it. >>>>> >>>>> I agree that changing I2C and SPI core is not trivial, however this is >>>>> no reason for piling up workarounds in all drivers. Are you seriously >>>>> advocating going though *every* driver and copying OF data into I2C/SPI >>>>> instead of doing the right thing and fixing the root of the issue? >>>> >>>> If you prefer to have this done (and I agree it would be a tiny improvement), >>>> please do it for us all. But please after merging this workaround. >>> >>> Have we been lucky to find someone who is able and willing to work on this? >>> >> >> As said, changing the core is trivial. A RFC patch is [0]. >> >> The problem is how to make sure that this change won't cause regressions >> in existing drivers. > > If the concern with drivers having I2C or SPI device table, but not OF > device table, then I think cocinnelle could be used to scan and find > them. > >> >> There was particularly tricky for the spi-nor driver, it doesn't help that >> a lot of DT are using undocumented compatible strings (sometimes with no >> vendor prefix). You can see the discussion here [1]. >> >> In the same thread Mark Brown said that it wasn't that bad to have the >> information in the OF device ID table duplicated in a SPI device table. >> >> Certainly isn't the best approach but IMHO is better than the module not >> loading. > > You can always build the module into kernel or load it by hand if it is > that important. Well, it is the only exception on the OpenPandora that does NOT yet load automatically from the DT. And sure we can always find a hack or workaround in user-space for everything the kernel isn't doing well. > Module autoloading does not work anyway if you have > several compatible strings, like > > compatible = "vendor,new-device", "vendor,generic-device"; We don't use that mechanism here since there is only one ads7846 installed and it won't change. BR, Nikolaus -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/input/touchscreen/ads7846.c b/drivers/input/touchscreen/ads7846.c index 400e421..50c85d2 100644 --- a/drivers/input/touchscreen/ads7846.c +++ b/drivers/input/touchscreen/ads7846.c @@ -1532,6 +1532,16 @@ static int ads7846_remove(struct spi_device *spi) return 0; } +static const struct spi_device_id ads7846_idtable[] = { + { "tsc2046", 0 }, + { "ads7843", 0 }, + { "ads7845", 0 }, + { "ads7846", 0 }, + { "ads7873", 0 }, + { } +}; +MODULE_DEVICE_TABLE(spi, ads7846_idtable); + static struct spi_driver ads7846_driver = { .driver = { .name = "ads7846", @@ -1546,4 +1556,3 @@ module_spi_driver(ads7846_driver); MODULE_DESCRIPTION("ADS7846 TouchScreen Driver"); MODULE_LICENSE("GPL"); -MODULE_ALIAS("spi:ads7846");
Fix module table so that the driver is loaded if compiled as module and requested by DT. Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com> --- drivers/input/touchscreen/ads7846.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)