2022-09-21 21:02:55

by Andy Shevchenko

[permalink] [raw]
Subject: [PATCH v1 1/1] spi: Introduce spi_get_device_match_data() helper

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 <[email protected]>
---
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)
{
--
2.35.1


2022-09-21 23:45:14

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH v1 1/1] spi: Introduce spi_get_device_match_data() helper

Hi Andy,

I love your patch! Yet something to improve:

[auto build test ERROR on broonie-spi/for-next]
[also build test ERROR 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: hexagon-randconfig-r045-20220921
compiler: clang version 16.0.0 (https://github.com/llvm/llvm-project 791a7ae1ba3efd6bca96338e10ffde557ba83920)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# 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=clang make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash drivers/spi/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>

All errors (new ones prefixed by >>):

>> drivers/spi/spi.c:367:32: error: passing 'const struct device *' to parameter of type 'struct device *' discards qualifiers [-Werror,-Wincompatible-pointer-types-discards-qualifiers]
match = device_get_match_data(&sdev->dev);
^~~~~~~~~~
include/linux/property.h:390:50: note: passing argument to parameter 'dev' here
const void *device_get_match_data(struct device *dev);
^
1 error generated.


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

--
0-DAY CI Kernel Test Service
https://01.org/lkp


Attachments:
(No filename) (2.59 kB)
config (152.61 kB)
Download all attachments

2022-09-22 04:16:49

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH v1 1/1] spi: Introduce spi_get_device_match_data() helper

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/[email protected]/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 <[email protected]>

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

--
0-DAY CI Kernel Test Service
https://01.org/lkp

2022-09-22 05:45:56

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH v1 1/1] spi: Introduce spi_get_device_match_data() helper

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/[email protected]/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 <[email protected]>

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

--
0-DAY CI Kernel Test Service
https://01.org/lkp

2022-09-22 11:28:00

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH v1 1/1] spi: Introduce spi_get_device_match_data() helper

On Wed, 21 Sep 2022 23:45:20 +0300
Andy Shevchenko <[email protected]> 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 <[email protected]>


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)
> {