* [PATCH] usb: max-3421: Use driver data instead of maintaining a list of bound devices
@ 2021-10-18 12:00 Uwe Kleine-König
2021-10-18 14:03 ` kernel test robot
2021-10-18 14:55 ` Greg Kroah-Hartman
0 siblings, 2 replies; 7+ messages in thread
From: Uwe Kleine-König @ 2021-10-18 12:00 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: David Mosberger, linux-usb, kernel
Instead of maintaining a single-linked list of devices that must be
searched linearly in .remove() just use spi_set_drvdata() to remember the
link between the spi device and the driver struct. Then the global list
and the next member can be dropped.
This simplifies the driver, reduces the memory footprint and the time to
search the list. Also it makes obvious that there is always a corresponding
driver struct for a given device in .remove(), so the error path for
!max3421_hcd can be dropped, too.
As a side effect this fixes a data inconsistency when .probe() races with
itself for a second max3421 device in manipulating max3421_hcd_list. A
similar race is fixed in .remove(), too.
Fixes: 2d53139f3162 ("Add support for using a MAX3421E chip as a host driver.")
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
drivers/usb/host/max3421-hcd.c | 26 +++++---------------------
1 file changed, 5 insertions(+), 21 deletions(-)
diff --git a/drivers/usb/host/max3421-hcd.c b/drivers/usb/host/max3421-hcd.c
index 59cc1bc7f12f..3e39f62904af 100644
--- a/drivers/usb/host/max3421-hcd.c
+++ b/drivers/usb/host/max3421-hcd.c
@@ -125,8 +125,6 @@ struct max3421_hcd {
struct task_struct *spi_thread;
- struct max3421_hcd *next;
-
enum max3421_rh_state rh_state;
/* lower 16 bits contain port status, upper 16 bits the change mask: */
u32 port_status;
@@ -174,8 +172,6 @@ struct max3421_ep {
u8 retransmit; /* packet needs retransmission */
};
-static struct max3421_hcd *max3421_hcd_list;
-
#define MAX3421_FIFO_SIZE 64
#define MAX3421_SPI_DIR_RD 0 /* read register from MAX3421 */
@@ -1881,10 +1877,8 @@ max3421_probe(struct spi_device *spi)
goto error;
}
set_bit(HCD_FLAG_POLL_RH, &hcd->flags);
- max3421_hcd = hcd_to_max3421(hcd);
- max3421_hcd->next = max3421_hcd_list;
- max3421_hcd_list = max3421_hcd;
INIT_LIST_HEAD(&max3421_hcd->ep_list);
+ spi_set_drvdata(spi, max3421_hcd);
max3421_hcd->tx = kmalloc(sizeof(*max3421_hcd->tx), GFP_KERNEL);
if (!max3421_hcd->tx)
@@ -1934,28 +1928,18 @@ max3421_probe(struct spi_device *spi)
static int
max3421_remove(struct spi_device *spi)
{
- struct max3421_hcd *max3421_hcd = NULL, **prev;
- struct usb_hcd *hcd = NULL;
+ struct max3421_hcd *max3421_hcd;
+ struct usb_hcd *hcd;
unsigned long flags;
- for (prev = &max3421_hcd_list; *prev; prev = &(*prev)->next) {
- max3421_hcd = *prev;
- hcd = max3421_to_hcd(max3421_hcd);
- if (hcd->self.controller == &spi->dev)
- break;
- }
- if (!max3421_hcd) {
- dev_err(&spi->dev, "no MAX3421 HCD found for SPI device %p\n",
- spi);
- return -ENODEV;
- }
+ max3421_hcd = spi_get_drvdata(spi);
+ hcd = max3421_to_hcd(max3421_hcd);
usb_remove_hcd(hcd);
spin_lock_irqsave(&max3421_hcd->lock, flags);
kthread_stop(max3421_hcd->spi_thread);
- *prev = max3421_hcd->next;
spin_unlock_irqrestore(&max3421_hcd->lock, flags);
--
2.30.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] usb: max-3421: Use driver data instead of maintaining a list of bound devices
2021-10-18 12:00 [PATCH] usb: max-3421: Use driver data instead of maintaining a list of bound devices Uwe Kleine-König
@ 2021-10-18 14:03 ` kernel test robot
2021-10-18 14:55 ` Greg Kroah-Hartman
1 sibling, 0 replies; 7+ messages in thread
From: kernel test robot @ 2021-10-18 14:03 UTC (permalink / raw)
To: Uwe Kleine-König, Greg Kroah-Hartman
Cc: llvm, kbuild-all, David Mosberger, linux-usb, kernel
[-- Attachment #1: Type: text/plain, Size: 10559 bytes --]
Hi "Uwe,
I love your patch! Perhaps something to improve:
[auto build test WARNING on usb/usb-testing]
[also build test WARNING on v5.15-rc6 next-20211018]
[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]
url: https://github.com/0day-ci/linux/commits/Uwe-Kleine-K-nig/usb-max-3421-Use-driver-data-instead-of-maintaining-a-list-of-bound-devices/20211018-200133
base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
config: i386-randconfig-r025-20211018 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project d245f2e8597bfb52c34810a328d42b990e4af1a4)
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/0day-ci/linux/commit/9ee7ff2f570093a02d9ae5e402a5becfdf87b9f5
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Uwe-Kleine-K-nig/usb-max-3421-Use-driver-data-instead-of-maintaining-a-list-of-bound-devices/20211018-200133
git checkout 9ee7ff2f570093a02d9ae5e402a5becfdf87b9f5
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 ARCH=i386
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> drivers/usb/host/max3421-hcd.c:1880:18: warning: variable 'max3421_hcd' is uninitialized when used here [-Wuninitialized]
INIT_LIST_HEAD(&max3421_hcd->ep_list);
^~~~~~~~~~~
drivers/usb/host/max3421-hcd.c:1827:33: note: initialize the variable 'max3421_hcd' to silence this warning
struct max3421_hcd *max3421_hcd;
^
= NULL
1 warning generated.
vim +/max3421_hcd +1880 drivers/usb/host/max3421-hcd.c
721fdc83b31b1b Jules Maselbas 2017-09-15 1822
2d53139f31626b David Mosberger 2014-04-28 1823 static int
2d53139f31626b David Mosberger 2014-04-28 1824 max3421_probe(struct spi_device *spi)
2d53139f31626b David Mosberger 2014-04-28 1825 {
721fdc83b31b1b Jules Maselbas 2017-09-15 1826 struct device *dev = &spi->dev;
2d53139f31626b David Mosberger 2014-04-28 1827 struct max3421_hcd *max3421_hcd;
05dfa5c9bc3793 David Mosberger-Tang 2014-05-28 1828 struct usb_hcd *hcd = NULL;
721fdc83b31b1b Jules Maselbas 2017-09-15 1829 struct max3421_hcd_platform_data *pdata = NULL;
5a569343e8a618 Yang Yingliang 2020-11-17 1830 int retval;
2d53139f31626b David Mosberger 2014-04-28 1831
2d53139f31626b David Mosberger 2014-04-28 1832 if (spi_setup(spi) < 0) {
2d53139f31626b David Mosberger 2014-04-28 1833 dev_err(&spi->dev, "Unable to setup SPI bus");
2d53139f31626b David Mosberger 2014-04-28 1834 return -EFAULT;
2d53139f31626b David Mosberger 2014-04-28 1835 }
2d53139f31626b David Mosberger 2014-04-28 1836
721fdc83b31b1b Jules Maselbas 2017-09-15 1837 if (!spi->irq) {
721fdc83b31b1b Jules Maselbas 2017-09-15 1838 dev_err(dev, "Failed to get SPI IRQ");
721fdc83b31b1b Jules Maselbas 2017-09-15 1839 return -EFAULT;
721fdc83b31b1b Jules Maselbas 2017-09-15 1840 }
721fdc83b31b1b Jules Maselbas 2017-09-15 1841
721fdc83b31b1b Jules Maselbas 2017-09-15 1842 if (IS_ENABLED(CONFIG_OF) && dev->of_node) {
721fdc83b31b1b Jules Maselbas 2017-09-15 1843 pdata = devm_kzalloc(&spi->dev, sizeof(*pdata), GFP_KERNEL);
721fdc83b31b1b Jules Maselbas 2017-09-15 1844 if (!pdata) {
721fdc83b31b1b Jules Maselbas 2017-09-15 1845 retval = -ENOMEM;
721fdc83b31b1b Jules Maselbas 2017-09-15 1846 goto error;
721fdc83b31b1b Jules Maselbas 2017-09-15 1847 }
721fdc83b31b1b Jules Maselbas 2017-09-15 1848 retval = max3421_of_vbus_en_pin(dev, pdata);
721fdc83b31b1b Jules Maselbas 2017-09-15 1849 if (retval)
721fdc83b31b1b Jules Maselbas 2017-09-15 1850 goto error;
721fdc83b31b1b Jules Maselbas 2017-09-15 1851
721fdc83b31b1b Jules Maselbas 2017-09-15 1852 spi->dev.platform_data = pdata;
721fdc83b31b1b Jules Maselbas 2017-09-15 1853 }
721fdc83b31b1b Jules Maselbas 2017-09-15 1854
721fdc83b31b1b Jules Maselbas 2017-09-15 1855 pdata = spi->dev.platform_data;
721fdc83b31b1b Jules Maselbas 2017-09-15 1856 if (!pdata) {
721fdc83b31b1b Jules Maselbas 2017-09-15 1857 dev_err(&spi->dev, "driver configuration data is not provided\n");
721fdc83b31b1b Jules Maselbas 2017-09-15 1858 retval = -EFAULT;
721fdc83b31b1b Jules Maselbas 2017-09-15 1859 goto error;
721fdc83b31b1b Jules Maselbas 2017-09-15 1860 }
721fdc83b31b1b Jules Maselbas 2017-09-15 1861 if (pdata->vbus_active_level > 1) {
721fdc83b31b1b Jules Maselbas 2017-09-15 1862 dev_err(&spi->dev, "vbus active level value %d is out of range (0/1)\n", pdata->vbus_active_level);
721fdc83b31b1b Jules Maselbas 2017-09-15 1863 retval = -EINVAL;
721fdc83b31b1b Jules Maselbas 2017-09-15 1864 goto error;
721fdc83b31b1b Jules Maselbas 2017-09-15 1865 }
721fdc83b31b1b Jules Maselbas 2017-09-15 1866 if (pdata->vbus_gpout < 1 || pdata->vbus_gpout > MAX3421_GPOUT_COUNT) {
721fdc83b31b1b Jules Maselbas 2017-09-15 1867 dev_err(&spi->dev, "vbus gpout value %d is out of range (1..8)\n", pdata->vbus_gpout);
721fdc83b31b1b Jules Maselbas 2017-09-15 1868 retval = -EINVAL;
721fdc83b31b1b Jules Maselbas 2017-09-15 1869 goto error;
721fdc83b31b1b Jules Maselbas 2017-09-15 1870 }
721fdc83b31b1b Jules Maselbas 2017-09-15 1871
5a569343e8a618 Yang Yingliang 2020-11-17 1872 retval = -ENOMEM;
2d53139f31626b David Mosberger 2014-04-28 1873 hcd = usb_create_hcd(&max3421_hcd_desc, &spi->dev,
2d53139f31626b David Mosberger 2014-04-28 1874 dev_name(&spi->dev));
2d53139f31626b David Mosberger 2014-04-28 1875 if (!hcd) {
2d53139f31626b David Mosberger 2014-04-28 1876 dev_err(&spi->dev, "failed to create HCD structure\n");
05dfa5c9bc3793 David Mosberger-Tang 2014-05-28 1877 goto error;
2d53139f31626b David Mosberger 2014-04-28 1878 }
2d53139f31626b David Mosberger 2014-04-28 1879 set_bit(HCD_FLAG_POLL_RH, &hcd->flags);
2d53139f31626b David Mosberger 2014-04-28 @1880 INIT_LIST_HEAD(&max3421_hcd->ep_list);
9ee7ff2f570093 Uwe Kleine-König 2021-10-18 1881 spi_set_drvdata(spi, max3421_hcd);
2d53139f31626b David Mosberger 2014-04-28 1882
05dfa5c9bc3793 David Mosberger-Tang 2014-05-28 1883 max3421_hcd->tx = kmalloc(sizeof(*max3421_hcd->tx), GFP_KERNEL);
13dcf780059417 Wolfram Sang 2016-08-25 1884 if (!max3421_hcd->tx)
05dfa5c9bc3793 David Mosberger-Tang 2014-05-28 1885 goto error;
05dfa5c9bc3793 David Mosberger-Tang 2014-05-28 1886 max3421_hcd->rx = kmalloc(sizeof(*max3421_hcd->rx), GFP_KERNEL);
13dcf780059417 Wolfram Sang 2016-08-25 1887 if (!max3421_hcd->rx)
05dfa5c9bc3793 David Mosberger-Tang 2014-05-28 1888 goto error;
05dfa5c9bc3793 David Mosberger-Tang 2014-05-28 1889
2d53139f31626b David Mosberger 2014-04-28 1890 max3421_hcd->spi_thread = kthread_run(max3421_spi_thread, hcd,
2d53139f31626b David Mosberger 2014-04-28 1891 "max3421_spi_thread");
2d53139f31626b David Mosberger 2014-04-28 1892 if (max3421_hcd->spi_thread == ERR_PTR(-ENOMEM)) {
2d53139f31626b David Mosberger 2014-04-28 1893 dev_err(&spi->dev,
2d53139f31626b David Mosberger 2014-04-28 1894 "failed to create SPI thread (out of memory)\n");
05dfa5c9bc3793 David Mosberger-Tang 2014-05-28 1895 goto error;
2d53139f31626b David Mosberger 2014-04-28 1896 }
2d53139f31626b David Mosberger 2014-04-28 1897
2d53139f31626b David Mosberger 2014-04-28 1898 retval = usb_add_hcd(hcd, 0, 0);
2d53139f31626b David Mosberger 2014-04-28 1899 if (retval) {
2d53139f31626b David Mosberger 2014-04-28 1900 dev_err(&spi->dev, "failed to add HCD\n");
05dfa5c9bc3793 David Mosberger-Tang 2014-05-28 1901 goto error;
2d53139f31626b David Mosberger 2014-04-28 1902 }
2d53139f31626b David Mosberger 2014-04-28 1903
2d53139f31626b David Mosberger 2014-04-28 1904 retval = request_irq(spi->irq, max3421_irq_handler,
2d53139f31626b David Mosberger 2014-04-28 1905 IRQF_TRIGGER_LOW, "max3421", hcd);
2d53139f31626b David Mosberger 2014-04-28 1906 if (retval < 0) {
2d53139f31626b David Mosberger 2014-04-28 1907 dev_err(&spi->dev, "failed to request irq %d\n", spi->irq);
05dfa5c9bc3793 David Mosberger-Tang 2014-05-28 1908 goto error;
2d53139f31626b David Mosberger 2014-04-28 1909 }
2d53139f31626b David Mosberger 2014-04-28 1910 return 0;
05dfa5c9bc3793 David Mosberger-Tang 2014-05-28 1911
05dfa5c9bc3793 David Mosberger-Tang 2014-05-28 1912 error:
721fdc83b31b1b Jules Maselbas 2017-09-15 1913 if (IS_ENABLED(CONFIG_OF) && dev->of_node && pdata) {
721fdc83b31b1b Jules Maselbas 2017-09-15 1914 devm_kfree(&spi->dev, pdata);
721fdc83b31b1b Jules Maselbas 2017-09-15 1915 spi->dev.platform_data = NULL;
721fdc83b31b1b Jules Maselbas 2017-09-15 1916 }
721fdc83b31b1b Jules Maselbas 2017-09-15 1917
05dfa5c9bc3793 David Mosberger-Tang 2014-05-28 1918 if (hcd) {
05dfa5c9bc3793 David Mosberger-Tang 2014-05-28 1919 kfree(max3421_hcd->tx);
05dfa5c9bc3793 David Mosberger-Tang 2014-05-28 1920 kfree(max3421_hcd->rx);
05dfa5c9bc3793 David Mosberger-Tang 2014-05-28 1921 if (max3421_hcd->spi_thread)
05dfa5c9bc3793 David Mosberger-Tang 2014-05-28 1922 kthread_stop(max3421_hcd->spi_thread);
05dfa5c9bc3793 David Mosberger-Tang 2014-05-28 1923 usb_put_hcd(hcd);
05dfa5c9bc3793 David Mosberger-Tang 2014-05-28 1924 }
05dfa5c9bc3793 David Mosberger-Tang 2014-05-28 1925 return retval;
2d53139f31626b David Mosberger 2014-04-28 1926 }
2d53139f31626b David Mosberger 2014-04-28 1927
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 39324 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] usb: max-3421: Use driver data instead of maintaining a list of bound devices
@ 2021-10-18 14:03 ` kernel test robot
0 siblings, 0 replies; 7+ messages in thread
From: kernel test robot @ 2021-10-18 14:03 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 10713 bytes --]
Hi "Uwe,
I love your patch! Perhaps something to improve:
[auto build test WARNING on usb/usb-testing]
[also build test WARNING on v5.15-rc6 next-20211018]
[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]
url: https://github.com/0day-ci/linux/commits/Uwe-Kleine-K-nig/usb-max-3421-Use-driver-data-instead-of-maintaining-a-list-of-bound-devices/20211018-200133
base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
config: i386-randconfig-r025-20211018 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project d245f2e8597bfb52c34810a328d42b990e4af1a4)
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/0day-ci/linux/commit/9ee7ff2f570093a02d9ae5e402a5becfdf87b9f5
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Uwe-Kleine-K-nig/usb-max-3421-Use-driver-data-instead-of-maintaining-a-list-of-bound-devices/20211018-200133
git checkout 9ee7ff2f570093a02d9ae5e402a5becfdf87b9f5
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 ARCH=i386
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> drivers/usb/host/max3421-hcd.c:1880:18: warning: variable 'max3421_hcd' is uninitialized when used here [-Wuninitialized]
INIT_LIST_HEAD(&max3421_hcd->ep_list);
^~~~~~~~~~~
drivers/usb/host/max3421-hcd.c:1827:33: note: initialize the variable 'max3421_hcd' to silence this warning
struct max3421_hcd *max3421_hcd;
^
= NULL
1 warning generated.
vim +/max3421_hcd +1880 drivers/usb/host/max3421-hcd.c
721fdc83b31b1b Jules Maselbas 2017-09-15 1822
2d53139f31626b David Mosberger 2014-04-28 1823 static int
2d53139f31626b David Mosberger 2014-04-28 1824 max3421_probe(struct spi_device *spi)
2d53139f31626b David Mosberger 2014-04-28 1825 {
721fdc83b31b1b Jules Maselbas 2017-09-15 1826 struct device *dev = &spi->dev;
2d53139f31626b David Mosberger 2014-04-28 1827 struct max3421_hcd *max3421_hcd;
05dfa5c9bc3793 David Mosberger-Tang 2014-05-28 1828 struct usb_hcd *hcd = NULL;
721fdc83b31b1b Jules Maselbas 2017-09-15 1829 struct max3421_hcd_platform_data *pdata = NULL;
5a569343e8a618 Yang Yingliang 2020-11-17 1830 int retval;
2d53139f31626b David Mosberger 2014-04-28 1831
2d53139f31626b David Mosberger 2014-04-28 1832 if (spi_setup(spi) < 0) {
2d53139f31626b David Mosberger 2014-04-28 1833 dev_err(&spi->dev, "Unable to setup SPI bus");
2d53139f31626b David Mosberger 2014-04-28 1834 return -EFAULT;
2d53139f31626b David Mosberger 2014-04-28 1835 }
2d53139f31626b David Mosberger 2014-04-28 1836
721fdc83b31b1b Jules Maselbas 2017-09-15 1837 if (!spi->irq) {
721fdc83b31b1b Jules Maselbas 2017-09-15 1838 dev_err(dev, "Failed to get SPI IRQ");
721fdc83b31b1b Jules Maselbas 2017-09-15 1839 return -EFAULT;
721fdc83b31b1b Jules Maselbas 2017-09-15 1840 }
721fdc83b31b1b Jules Maselbas 2017-09-15 1841
721fdc83b31b1b Jules Maselbas 2017-09-15 1842 if (IS_ENABLED(CONFIG_OF) && dev->of_node) {
721fdc83b31b1b Jules Maselbas 2017-09-15 1843 pdata = devm_kzalloc(&spi->dev, sizeof(*pdata), GFP_KERNEL);
721fdc83b31b1b Jules Maselbas 2017-09-15 1844 if (!pdata) {
721fdc83b31b1b Jules Maselbas 2017-09-15 1845 retval = -ENOMEM;
721fdc83b31b1b Jules Maselbas 2017-09-15 1846 goto error;
721fdc83b31b1b Jules Maselbas 2017-09-15 1847 }
721fdc83b31b1b Jules Maselbas 2017-09-15 1848 retval = max3421_of_vbus_en_pin(dev, pdata);
721fdc83b31b1b Jules Maselbas 2017-09-15 1849 if (retval)
721fdc83b31b1b Jules Maselbas 2017-09-15 1850 goto error;
721fdc83b31b1b Jules Maselbas 2017-09-15 1851
721fdc83b31b1b Jules Maselbas 2017-09-15 1852 spi->dev.platform_data = pdata;
721fdc83b31b1b Jules Maselbas 2017-09-15 1853 }
721fdc83b31b1b Jules Maselbas 2017-09-15 1854
721fdc83b31b1b Jules Maselbas 2017-09-15 1855 pdata = spi->dev.platform_data;
721fdc83b31b1b Jules Maselbas 2017-09-15 1856 if (!pdata) {
721fdc83b31b1b Jules Maselbas 2017-09-15 1857 dev_err(&spi->dev, "driver configuration data is not provided\n");
721fdc83b31b1b Jules Maselbas 2017-09-15 1858 retval = -EFAULT;
721fdc83b31b1b Jules Maselbas 2017-09-15 1859 goto error;
721fdc83b31b1b Jules Maselbas 2017-09-15 1860 }
721fdc83b31b1b Jules Maselbas 2017-09-15 1861 if (pdata->vbus_active_level > 1) {
721fdc83b31b1b Jules Maselbas 2017-09-15 1862 dev_err(&spi->dev, "vbus active level value %d is out of range (0/1)\n", pdata->vbus_active_level);
721fdc83b31b1b Jules Maselbas 2017-09-15 1863 retval = -EINVAL;
721fdc83b31b1b Jules Maselbas 2017-09-15 1864 goto error;
721fdc83b31b1b Jules Maselbas 2017-09-15 1865 }
721fdc83b31b1b Jules Maselbas 2017-09-15 1866 if (pdata->vbus_gpout < 1 || pdata->vbus_gpout > MAX3421_GPOUT_COUNT) {
721fdc83b31b1b Jules Maselbas 2017-09-15 1867 dev_err(&spi->dev, "vbus gpout value %d is out of range (1..8)\n", pdata->vbus_gpout);
721fdc83b31b1b Jules Maselbas 2017-09-15 1868 retval = -EINVAL;
721fdc83b31b1b Jules Maselbas 2017-09-15 1869 goto error;
721fdc83b31b1b Jules Maselbas 2017-09-15 1870 }
721fdc83b31b1b Jules Maselbas 2017-09-15 1871
5a569343e8a618 Yang Yingliang 2020-11-17 1872 retval = -ENOMEM;
2d53139f31626b David Mosberger 2014-04-28 1873 hcd = usb_create_hcd(&max3421_hcd_desc, &spi->dev,
2d53139f31626b David Mosberger 2014-04-28 1874 dev_name(&spi->dev));
2d53139f31626b David Mosberger 2014-04-28 1875 if (!hcd) {
2d53139f31626b David Mosberger 2014-04-28 1876 dev_err(&spi->dev, "failed to create HCD structure\n");
05dfa5c9bc3793 David Mosberger-Tang 2014-05-28 1877 goto error;
2d53139f31626b David Mosberger 2014-04-28 1878 }
2d53139f31626b David Mosberger 2014-04-28 1879 set_bit(HCD_FLAG_POLL_RH, &hcd->flags);
2d53139f31626b David Mosberger 2014-04-28 @1880 INIT_LIST_HEAD(&max3421_hcd->ep_list);
9ee7ff2f570093 Uwe Kleine-König 2021-10-18 1881 spi_set_drvdata(spi, max3421_hcd);
2d53139f31626b David Mosberger 2014-04-28 1882
05dfa5c9bc3793 David Mosberger-Tang 2014-05-28 1883 max3421_hcd->tx = kmalloc(sizeof(*max3421_hcd->tx), GFP_KERNEL);
13dcf780059417 Wolfram Sang 2016-08-25 1884 if (!max3421_hcd->tx)
05dfa5c9bc3793 David Mosberger-Tang 2014-05-28 1885 goto error;
05dfa5c9bc3793 David Mosberger-Tang 2014-05-28 1886 max3421_hcd->rx = kmalloc(sizeof(*max3421_hcd->rx), GFP_KERNEL);
13dcf780059417 Wolfram Sang 2016-08-25 1887 if (!max3421_hcd->rx)
05dfa5c9bc3793 David Mosberger-Tang 2014-05-28 1888 goto error;
05dfa5c9bc3793 David Mosberger-Tang 2014-05-28 1889
2d53139f31626b David Mosberger 2014-04-28 1890 max3421_hcd->spi_thread = kthread_run(max3421_spi_thread, hcd,
2d53139f31626b David Mosberger 2014-04-28 1891 "max3421_spi_thread");
2d53139f31626b David Mosberger 2014-04-28 1892 if (max3421_hcd->spi_thread == ERR_PTR(-ENOMEM)) {
2d53139f31626b David Mosberger 2014-04-28 1893 dev_err(&spi->dev,
2d53139f31626b David Mosberger 2014-04-28 1894 "failed to create SPI thread (out of memory)\n");
05dfa5c9bc3793 David Mosberger-Tang 2014-05-28 1895 goto error;
2d53139f31626b David Mosberger 2014-04-28 1896 }
2d53139f31626b David Mosberger 2014-04-28 1897
2d53139f31626b David Mosberger 2014-04-28 1898 retval = usb_add_hcd(hcd, 0, 0);
2d53139f31626b David Mosberger 2014-04-28 1899 if (retval) {
2d53139f31626b David Mosberger 2014-04-28 1900 dev_err(&spi->dev, "failed to add HCD\n");
05dfa5c9bc3793 David Mosberger-Tang 2014-05-28 1901 goto error;
2d53139f31626b David Mosberger 2014-04-28 1902 }
2d53139f31626b David Mosberger 2014-04-28 1903
2d53139f31626b David Mosberger 2014-04-28 1904 retval = request_irq(spi->irq, max3421_irq_handler,
2d53139f31626b David Mosberger 2014-04-28 1905 IRQF_TRIGGER_LOW, "max3421", hcd);
2d53139f31626b David Mosberger 2014-04-28 1906 if (retval < 0) {
2d53139f31626b David Mosberger 2014-04-28 1907 dev_err(&spi->dev, "failed to request irq %d\n", spi->irq);
05dfa5c9bc3793 David Mosberger-Tang 2014-05-28 1908 goto error;
2d53139f31626b David Mosberger 2014-04-28 1909 }
2d53139f31626b David Mosberger 2014-04-28 1910 return 0;
05dfa5c9bc3793 David Mosberger-Tang 2014-05-28 1911
05dfa5c9bc3793 David Mosberger-Tang 2014-05-28 1912 error:
721fdc83b31b1b Jules Maselbas 2017-09-15 1913 if (IS_ENABLED(CONFIG_OF) && dev->of_node && pdata) {
721fdc83b31b1b Jules Maselbas 2017-09-15 1914 devm_kfree(&spi->dev, pdata);
721fdc83b31b1b Jules Maselbas 2017-09-15 1915 spi->dev.platform_data = NULL;
721fdc83b31b1b Jules Maselbas 2017-09-15 1916 }
721fdc83b31b1b Jules Maselbas 2017-09-15 1917
05dfa5c9bc3793 David Mosberger-Tang 2014-05-28 1918 if (hcd) {
05dfa5c9bc3793 David Mosberger-Tang 2014-05-28 1919 kfree(max3421_hcd->tx);
05dfa5c9bc3793 David Mosberger-Tang 2014-05-28 1920 kfree(max3421_hcd->rx);
05dfa5c9bc3793 David Mosberger-Tang 2014-05-28 1921 if (max3421_hcd->spi_thread)
05dfa5c9bc3793 David Mosberger-Tang 2014-05-28 1922 kthread_stop(max3421_hcd->spi_thread);
05dfa5c9bc3793 David Mosberger-Tang 2014-05-28 1923 usb_put_hcd(hcd);
05dfa5c9bc3793 David Mosberger-Tang 2014-05-28 1924 }
05dfa5c9bc3793 David Mosberger-Tang 2014-05-28 1925 return retval;
2d53139f31626b David Mosberger 2014-04-28 1926 }
2d53139f31626b David Mosberger 2014-04-28 1927
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 39324 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] usb: max-3421: Use driver data instead of maintaining a list of bound devices
2021-10-18 12:00 [PATCH] usb: max-3421: Use driver data instead of maintaining a list of bound devices Uwe Kleine-König
2021-10-18 14:03 ` kernel test robot
@ 2021-10-18 14:55 ` Greg Kroah-Hartman
2021-10-18 20:28 ` Uwe Kleine-König
1 sibling, 1 reply; 7+ messages in thread
From: Greg Kroah-Hartman @ 2021-10-18 14:55 UTC (permalink / raw)
To: Uwe Kleine-König; +Cc: David Mosberger, linux-usb, kernel
On Mon, Oct 18, 2021 at 02:00:55PM +0200, Uwe Kleine-König wrote:
> Instead of maintaining a single-linked list of devices that must be
> searched linearly in .remove() just use spi_set_drvdata() to remember the
> link between the spi device and the driver struct. Then the global list
> and the next member can be dropped.
>
> This simplifies the driver, reduces the memory footprint and the time to
> search the list. Also it makes obvious that there is always a corresponding
> driver struct for a given device in .remove(), so the error path for
> !max3421_hcd can be dropped, too.
>
> As a side effect this fixes a data inconsistency when .probe() races with
> itself for a second max3421 device in manipulating max3421_hcd_list. A
> similar race is fixed in .remove(), too.
>
> Fixes: 2d53139f3162 ("Add support for using a MAX3421E chip as a host driver.")
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
> drivers/usb/host/max3421-hcd.c | 26 +++++---------------------
> 1 file changed, 5 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/usb/host/max3421-hcd.c b/drivers/usb/host/max3421-hcd.c
> index 59cc1bc7f12f..3e39f62904af 100644
> --- a/drivers/usb/host/max3421-hcd.c
> +++ b/drivers/usb/host/max3421-hcd.c
> @@ -125,8 +125,6 @@ struct max3421_hcd {
>
> struct task_struct *spi_thread;
>
> - struct max3421_hcd *next;
> -
> enum max3421_rh_state rh_state;
> /* lower 16 bits contain port status, upper 16 bits the change mask: */
> u32 port_status;
> @@ -174,8 +172,6 @@ struct max3421_ep {
> u8 retransmit; /* packet needs retransmission */
> };
>
> -static struct max3421_hcd *max3421_hcd_list;
> -
> #define MAX3421_FIFO_SIZE 64
>
> #define MAX3421_SPI_DIR_RD 0 /* read register from MAX3421 */
> @@ -1881,10 +1877,8 @@ max3421_probe(struct spi_device *spi)
> goto error;
> }
> set_bit(HCD_FLAG_POLL_RH, &hcd->flags);
> - max3421_hcd = hcd_to_max3421(hcd);
I don't think you should have deleted this line :(
Did you test this?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] usb: max-3421: Use driver data instead of maintaining a list of bound devices
2021-10-18 14:55 ` Greg Kroah-Hartman
@ 2021-10-18 20:28 ` Uwe Kleine-König
2021-10-18 20:40 ` [PATCH v2] " Uwe Kleine-König
0 siblings, 1 reply; 7+ messages in thread
From: Uwe Kleine-König @ 2021-10-18 20:28 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: David Mosberger, linux-usb, kernel
[-- Attachment #1: Type: text/plain, Size: 726 bytes --]
Hello Greg,
On Mon, Oct 18, 2021 at 04:55:38PM +0200, Greg Kroah-Hartman wrote:
> On Mon, Oct 18, 2021 at 02:00:55PM +0200, Uwe Kleine-König wrote:
> > @@ -1881,10 +1877,8 @@ max3421_probe(struct spi_device *spi)
> > goto error;
> > }
> > set_bit(HCD_FLAG_POLL_RH, &hcd->flags);
> > - max3421_hcd = hcd_to_max3421(hcd);
>
> I don't think you should have deleted this line :(
>
> Did you test this?
Only compile tested (and the warning the kernel robot found didn't
happen in my amd64 build).
I'll respin this patch.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | https://www.pengutronix.de/ |
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2] usb: max-3421: Use driver data instead of maintaining a list of bound devices
2021-10-18 20:28 ` Uwe Kleine-König
@ 2021-10-18 20:40 ` Uwe Kleine-König
2021-10-19 14:51 ` David Mosberger-Tang
0 siblings, 1 reply; 7+ messages in thread
From: Uwe Kleine-König @ 2021-10-18 20:40 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: David Mosberger, linux-usb, kernel
Instead of maintaining a single-linked list of devices that must be
searched linearly in .remove() just use spi_set_drvdata() to remember the
link between the spi device and the driver struct. Then the global list
and the next member can be dropped.
This simplifies the driver, reduces the memory footprint and the time to
search the list. Also it makes obvious that there is always a corresponding
driver struct for a given device in .remove(), so the error path for
!max3421_hcd can be dropped, too.
As a side effect this fixes a data inconsistency when .probe() races with
itself for a second max3421 device in manipulating max3421_hcd_list. A
similar race is fixed in .remove(), too.
Fixes: 2d53139f3162 ("Add support for using a MAX3421E chip as a host driver.")
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
Changes since (implicit) v1:
- don't drop "max3421_hcd = hcd_to_max3421(hcd);", noticed by the
kernel test robot. Greg helped interpreting the kernel test robot's
finding.
As before, this patch is only build tested.
Best regards
Uwe
drivers/usb/host/max3421-hcd.c | 25 +++++--------------------
1 file changed, 5 insertions(+), 20 deletions(-)
diff --git a/drivers/usb/host/max3421-hcd.c b/drivers/usb/host/max3421-hcd.c
index 59cc1bc7f12f..30de85a707fe 100644
--- a/drivers/usb/host/max3421-hcd.c
+++ b/drivers/usb/host/max3421-hcd.c
@@ -125,8 +125,6 @@ struct max3421_hcd {
struct task_struct *spi_thread;
- struct max3421_hcd *next;
-
enum max3421_rh_state rh_state;
/* lower 16 bits contain port status, upper 16 bits the change mask: */
u32 port_status;
@@ -174,8 +172,6 @@ struct max3421_ep {
u8 retransmit; /* packet needs retransmission */
};
-static struct max3421_hcd *max3421_hcd_list;
-
#define MAX3421_FIFO_SIZE 64
#define MAX3421_SPI_DIR_RD 0 /* read register from MAX3421 */
@@ -1882,9 +1878,8 @@ max3421_probe(struct spi_device *spi)
}
set_bit(HCD_FLAG_POLL_RH, &hcd->flags);
max3421_hcd = hcd_to_max3421(hcd);
- max3421_hcd->next = max3421_hcd_list;
- max3421_hcd_list = max3421_hcd;
INIT_LIST_HEAD(&max3421_hcd->ep_list);
+ spi_set_drvdata(spi, max3421_hcd);
max3421_hcd->tx = kmalloc(sizeof(*max3421_hcd->tx), GFP_KERNEL);
if (!max3421_hcd->tx)
@@ -1934,28 +1929,18 @@ max3421_probe(struct spi_device *spi)
static int
max3421_remove(struct spi_device *spi)
{
- struct max3421_hcd *max3421_hcd = NULL, **prev;
- struct usb_hcd *hcd = NULL;
+ struct max3421_hcd *max3421_hcd;
+ struct usb_hcd *hcd;
unsigned long flags;
- for (prev = &max3421_hcd_list; *prev; prev = &(*prev)->next) {
- max3421_hcd = *prev;
- hcd = max3421_to_hcd(max3421_hcd);
- if (hcd->self.controller == &spi->dev)
- break;
- }
- if (!max3421_hcd) {
- dev_err(&spi->dev, "no MAX3421 HCD found for SPI device %p\n",
- spi);
- return -ENODEV;
- }
+ max3421_hcd = spi_get_drvdata(spi);
+ hcd = max3421_to_hcd(max3421_hcd);
usb_remove_hcd(hcd);
spin_lock_irqsave(&max3421_hcd->lock, flags);
kthread_stop(max3421_hcd->spi_thread);
- *prev = max3421_hcd->next;
spin_unlock_irqrestore(&max3421_hcd->lock, flags);
--
2.30.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2] usb: max-3421: Use driver data instead of maintaining a list of bound devices
2021-10-18 20:40 ` [PATCH v2] " Uwe Kleine-König
@ 2021-10-19 14:51 ` David Mosberger-Tang
0 siblings, 0 replies; 7+ messages in thread
From: David Mosberger-Tang @ 2021-10-19 14:51 UTC (permalink / raw)
To: Uwe Kleine-König, Greg Kroah-Hartman; +Cc: linux-usb, kernel
Looks fine to me. I could only compile-test though since I don't have
this hardware any more.
Ack-by: David Mosberger-Tang <davidm@egauge.net>
Thanks,
--david
On Mon, 2021-10-18 at 22:40 +0200, Uwe Kleine-König wrote:
> Instead of maintaining a single-linked list of devices that must be
> searched linearly in .remove() just use spi_set_drvdata() to remember the
> link between the spi device and the driver struct. Then the global list
> and the next member can be dropped.
>
> This simplifies the driver, reduces the memory footprint and the time to
> search the list. Also it makes obvious that there is always a corresponding
> driver struct for a given device in .remove(), so the error path for
> !max3421_hcd can be dropped, too.
>
> As a side effect this fixes a data inconsistency when .probe() races with
> itself for a second max3421 device in manipulating max3421_hcd_list. A
> similar race is fixed in .remove(), too.
>
> Fixes: 2d53139f3162 ("Add support for using a MAX3421E chip as a host driver.")
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
> Changes since (implicit) v1:
>
> - don't drop "max3421_hcd = hcd_to_max3421(hcd);", noticed by the
> kernel test robot. Greg helped interpreting the kernel test robot's
> finding.
>
> As before, this patch is only build tested.
>
> Best regards
> Uwe
> drivers/usb/host/max3421-hcd.c | 25 +++++--------------------
> 1 file changed, 5 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/usb/host/max3421-hcd.c b/drivers/usb/host/max3421-hcd.c
> index 59cc1bc7f12f..30de85a707fe 100644
> --- a/drivers/usb/host/max3421-hcd.c
> +++ b/drivers/usb/host/max3421-hcd.c
> @@ -125,8 +125,6 @@ struct max3421_hcd {
>
> struct task_struct *spi_thread;
>
> - struct max3421_hcd *next;
> -
> enum max3421_rh_state rh_state;
> /* lower 16 bits contain port status, upper 16 bits the change mask: */
> u32 port_status;
> @@ -174,8 +172,6 @@ struct max3421_ep {
> u8 retransmit; /* packet needs retransmission */
> };
>
> -static struct max3421_hcd *max3421_hcd_list;
> -
> #define MAX3421_FIFO_SIZE 64
>
> #define MAX3421_SPI_DIR_RD 0 /* read register from MAX3421 */
> @@ -1882,9 +1878,8 @@ max3421_probe(struct spi_device *spi)
> }
> set_bit(HCD_FLAG_POLL_RH, &hcd->flags);
> max3421_hcd = hcd_to_max3421(hcd);
> - max3421_hcd->next = max3421_hcd_list;
> - max3421_hcd_list = max3421_hcd;
> INIT_LIST_HEAD(&max3421_hcd->ep_list);
> + spi_set_drvdata(spi, max3421_hcd);
>
> max3421_hcd->tx = kmalloc(sizeof(*max3421_hcd->tx), GFP_KERNEL);
> if (!max3421_hcd->tx)
> @@ -1934,28 +1929,18 @@ max3421_probe(struct spi_device *spi)
> static int
> max3421_remove(struct spi_device *spi)
> {
> - struct max3421_hcd *max3421_hcd = NULL, **prev;
> - struct usb_hcd *hcd = NULL;
> + struct max3421_hcd *max3421_hcd;
> + struct usb_hcd *hcd;
> unsigned long flags;
>
> - for (prev = &max3421_hcd_list; *prev; prev = &(*prev)->next) {
> - max3421_hcd = *prev;
> - hcd = max3421_to_hcd(max3421_hcd);
> - if (hcd->self.controller == &spi->dev)
> - break;
> - }
> - if (!max3421_hcd) {
> - dev_err(&spi->dev, "no MAX3421 HCD found for SPI device %p\n",
> - spi);
> - return -ENODEV;
> - }
> + max3421_hcd = spi_get_drvdata(spi);
> + hcd = max3421_to_hcd(max3421_hcd);
>
> usb_remove_hcd(hcd);
>
> spin_lock_irqsave(&max3421_hcd->lock, flags);
>
> kthread_stop(max3421_hcd->spi_thread);
> - *prev = max3421_hcd->next;
>
> spin_unlock_irqrestore(&max3421_hcd->lock, flags);
>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-10-19 14:52 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-18 12:00 [PATCH] usb: max-3421: Use driver data instead of maintaining a list of bound devices Uwe Kleine-König
2021-10-18 14:03 ` kernel test robot
2021-10-18 14:03 ` kernel test robot
2021-10-18 14:55 ` Greg Kroah-Hartman
2021-10-18 20:28 ` Uwe Kleine-König
2021-10-18 20:40 ` [PATCH v2] " Uwe Kleine-König
2021-10-19 14:51 ` David Mosberger-Tang
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.