diff mbox series

[v1,1/1] spi: Introduce spi_get_device_match_data() helper

Message ID 20220921204520.23984-1-andriy.shevchenko@linux.intel.com (mailing list archive)
State Handled Elsewhere
Headers show
Series [v1,1/1] spi: Introduce spi_get_device_match_data() helper | expand

Commit Message

Andy Shevchenko Sept. 21, 2022, 8:45 p.m. UTC 
The proposed spi_get_device_match_data() helper is for retrieving
a driver data associated with the ID in an ID table. First, it tries
to get driver data of the device enumerated by firmware interface
(usually Device Tree or ACPI). If none is found it falls back to
the SPI ID table matching.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/spi/spi.c       | 12 ++++++++++++
 include/linux/spi/spi.h |  3 +++
 2 files changed, 15 insertions(+)

Comments

kernel test robot Sept. 22, 2022, 3:45 a.m. UTC | #1 
Hi Andy,

I love your patch! Perhaps something to improve:

[auto build test WARNING on broonie-spi/for-next]
[also build test WARNING on linus/master v6.0-rc6 next-20220921]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Andy-Shevchenko/spi-Introduce-spi_get_device_match_data-helper/20220922-044658
base:   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next
config: x86_64-randconfig-a013 (https://download.01.org/0day-ci/archive/20220922/202209221154.aGU73pNV-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-5) 11.3.0
reproduce (this is a W=1 build):
        # https://github.com/intel-lab-lkp/linux/commit/4d7a10e29738f98137b08b2dcc0297535dd92a11
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Andy-Shevchenko/spi-Introduce-spi_get_device_match_data-helper/20220922-044658
        git checkout 4d7a10e29738f98137b08b2dcc0297535dd92a11
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/spi/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   drivers/spi/spi.c: In function 'spi_get_device_match_data':
>> drivers/spi/spi.c:367:39: warning: passing argument 1 of 'device_get_match_data' discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
     367 |         match = device_get_match_data(&sdev->dev);
         |                                       ^~~~~~~~~~
   In file included from include/linux/of.h:22,
                    from include/linux/of_device.h:9,
                    from drivers/spi/spi.c:14:
   include/linux/property.h:390:50: note: expected 'struct device *' but argument is of type 'const struct device *'
     390 | const void *device_get_match_data(struct device *dev);
         |                                   ~~~~~~~~~~~~~~~^~~


vim +367 drivers/spi/spi.c

   362	
   363	const void *spi_get_device_match_data(const struct spi_device *sdev)
   364	{
   365		const void *match;
   366	
 > 367		match = device_get_match_data(&sdev->dev);
   368		if (match)
   369			return match;
   370	
   371		return (const void *)spi_get_device_id(sdev)->driver_data;
   372	}
   373	EXPORT_SYMBOL_GPL(spi_get_device_match_data);
   374
kernel test robot Sept. 22, 2022, 5:34 a.m. UTC | #2 
Hi Andy,

I love your patch! Perhaps something to improve:

[auto build test WARNING on broonie-spi/for-next]
[also build test WARNING on linus/master v6.0-rc6 next-20220921]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Andy-Shevchenko/spi-Introduce-spi_get_device_match_data-helper/20220922-044658
base:   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next
config: s390-randconfig-s031-20220921 (https://download.01.org/0day-ci/archive/20220922/202209221302.UKxElkNH-lkp@intel.com/config)
compiler: s390-linux-gcc (GCC) 12.1.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # apt-get install sparse
        # sparse version: v0.6.4-39-gce1a6720-dirty
        # https://github.com/intel-lab-lkp/linux/commit/4d7a10e29738f98137b08b2dcc0297535dd92a11
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Andy-Shevchenko/spi-Introduce-spi_get_device_match_data-helper/20220922-044658
        git checkout 4d7a10e29738f98137b08b2dcc0297535dd92a11
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=s390 SHELL=/bin/bash drivers/spi/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>

sparse warnings: (new ones prefixed by >>)
>> drivers/spi/spi.c:367:40: sparse: sparse: incorrect type in argument 1 (different modifiers) @@     expected struct device *dev @@     got struct device const * @@
   drivers/spi/spi.c:367:40: sparse:     expected struct device *dev
   drivers/spi/spi.c:367:40: sparse:     got struct device const *

vim +367 drivers/spi/spi.c

   362	
   363	const void *spi_get_device_match_data(const struct spi_device *sdev)
   364	{
   365		const void *match;
   366	
 > 367		match = device_get_match_data(&sdev->dev);
   368		if (match)
   369			return match;
   370	
   371		return (const void *)spi_get_device_id(sdev)->driver_data;
   372	}
   373	EXPORT_SYMBOL_GPL(spi_get_device_match_data);
   374
Jonathan Cameron Sept. 22, 2022, 11:18 a.m. UTC | #3 
On Wed, 21 Sep 2022 23:45:20 +0300
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> The proposed spi_get_device_match_data() helper is for retrieving
> a driver data associated with the ID in an ID table. First, it tries
> to get driver data of the device enumerated by firmware interface
> (usually Device Tree or ACPI). If none is found it falls back to
> the SPI ID table matching.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>


I like this in general, but we need to keep a close eye on usecases
that are introduced to make sure that there is alignment between the
contents of the tables.

Perhaps the text should also explain a bit about why we would allow
both paths (in many case, the fallback will work fine)...
Previously a strong reason for that was that there was nothing to ensure
that all dt table entries were mirrored in the spi_device_id table.
As a side effect of the check to ensure module autoloading works, there
is now a check for that.

My personal view is we should still use the more generic path, even if
magic under the hood puts the data in spi_id->driver_data.

Jonathan


> ---
>  drivers/spi/spi.c       | 12 ++++++++++++
>  include/linux/spi/spi.h |  3 +++
>  2 files changed, 15 insertions(+)
> 
> diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
> index ad254b94308e..a0947d63afbc 100644
> --- a/drivers/spi/spi.c
> +++ b/drivers/spi/spi.c
> @@ -360,6 +360,18 @@ const struct spi_device_id *spi_get_device_id(const struct spi_device *sdev)
>  }
>  EXPORT_SYMBOL_GPL(spi_get_device_id);
>  
> +const void *spi_get_device_match_data(const struct spi_device *sdev)
> +{
> +	const void *match;
> +
> +	match = device_get_match_data(&sdev->dev);
> +	if (match)
> +		return match;
> +
> +	return (const void *)spi_get_device_id(sdev)->driver_data;
> +}
> +EXPORT_SYMBOL_GPL(spi_get_device_match_data);
> +
>  static int spi_match_device(struct device *dev, struct device_driver *drv)
>  {
>  	const struct spi_device	*spi = to_spi_device(dev);
> diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
> index 6ea889df0813..f2565c24ef27 100644
> --- a/include/linux/spi/spi.h
> +++ b/include/linux/spi/spi.h
> @@ -1510,6 +1510,9 @@ extern void spi_unregister_device(struct spi_device *spi);
>  extern const struct spi_device_id *
>  spi_get_device_id(const struct spi_device *sdev);
>  
> +extern const void *
> +spi_get_device_match_data(const struct spi_device *sdev);
> +
>  static inline bool
>  spi_transfer_is_last(struct spi_controller *ctlr, struct spi_transfer *xfer)
>  {
diff mbox series

Patch

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index ad254b94308e..a0947d63afbc 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -360,6 +360,18 @@  const struct spi_device_id *spi_get_device_id(const struct spi_device *sdev)
 }
 EXPORT_SYMBOL_GPL(spi_get_device_id);
 
+const void *spi_get_device_match_data(const struct spi_device *sdev)
+{
+	const void *match;
+
+	match = device_get_match_data(&sdev->dev);
+	if (match)
+		return match;
+
+	return (const void *)spi_get_device_id(sdev)->driver_data;
+}
+EXPORT_SYMBOL_GPL(spi_get_device_match_data);
+
 static int spi_match_device(struct device *dev, struct device_driver *drv)
 {
 	const struct spi_device	*spi = to_spi_device(dev);
diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index 6ea889df0813..f2565c24ef27 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -1510,6 +1510,9 @@  extern void spi_unregister_device(struct spi_device *spi);
 extern const struct spi_device_id *
 spi_get_device_id(const struct spi_device *sdev);
 
+extern const void *
+spi_get_device_match_data(const struct spi_device *sdev);
+
 static inline bool
 spi_transfer_is_last(struct spi_controller *ctlr, struct spi_transfer *xfer)
 {