| Message ID | 2e727b49-b59b-fad9-6ed3-febc5b98b8d4@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | i2c: i801: Series with improvements | expand |
On Sun, 01 Aug 2021 16:18:38 +0200, Heiner Kallweit wrote: > p2sb_spinlock is used in i801_add_tco_spt() only and in process context > only. Therefore a mutex is sufficient, and we can make the definition > local to i801_add_tco_spt(). Mika, no objection? > Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> > --- > drivers/i2c/busses/i2c-i801.c | 7 +++---- > 1 file changed, 3 insertions(+), 4 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c > index bdb619bc0..504f02e1e 100644 > --- a/drivers/i2c/busses/i2c-i801.c > +++ b/drivers/i2c/busses/i2c-i801.c > @@ -1492,12 +1492,11 @@ static const struct itco_wdt_platform_data spt_tco_platform_data = { > .version = 4, > }; > > -static DEFINE_SPINLOCK(p2sb_spinlock); > - > static struct platform_device * > i801_add_tco_spt(struct i801_priv *priv, struct pci_dev *pci_dev, > struct resource *tco_res) > { > + static DEFINE_MUTEX(p2sb_mutex); To be on the safe side, we should explicitly #include <linux/mutex.h>. > struct resource *res; > unsigned int devfn; > u64 base64_addr; > @@ -1510,7 +1509,7 @@ i801_add_tco_spt(struct i801_priv *priv, struct pci_dev *pci_dev, > * enumerated by the PCI subsystem, so we need to unhide/hide it > * to lookup the P2SB BAR. > */ > - spin_lock(&p2sb_spinlock); > + mutex_lock(&p2sb_mutex); > > devfn = PCI_DEVFN(PCI_SLOT(pci_dev->devfn), 1); > > @@ -1528,7 +1527,7 @@ i801_add_tco_spt(struct i801_priv *priv, struct pci_dev *pci_dev, > /* Hide the P2SB device, if it was hidden before */ > if (hidden) > pci_bus_write_config_byte(pci_dev->bus, devfn, 0xe1, hidden); > - spin_unlock(&p2sb_spinlock); > + mutex_unlock(&p2sb_mutex); > > res = &tco_res[1]; > if (pci_dev->device == PCI_DEVICE_ID_INTEL_DNV_SMBUS) Reviewed-by: Jean Delvare <jdelvare@suse.de>
On Thu, Aug 05, 2021 at 10:49:39AM +0200, Jean Delvare wrote: > On Sun, 01 Aug 2021 16:18:38 +0200, Heiner Kallweit wrote: > > p2sb_spinlock is used in i801_add_tco_spt() only and in process context > > only. Therefore a mutex is sufficient, and we can make the definition > > local to i801_add_tco_spt(). > > Mika, no objection? No objections from my side :) Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com> > > Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> > > --- > > drivers/i2c/busses/i2c-i801.c | 7 +++---- > > 1 file changed, 3 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c > > index bdb619bc0..504f02e1e 100644 > > --- a/drivers/i2c/busses/i2c-i801.c > > +++ b/drivers/i2c/busses/i2c-i801.c > > @@ -1492,12 +1492,11 @@ static const struct itco_wdt_platform_data spt_tco_platform_data = { > > .version = 4, > > }; > > > > -static DEFINE_SPINLOCK(p2sb_spinlock); > > - > > static struct platform_device * > > i801_add_tco_spt(struct i801_priv *priv, struct pci_dev *pci_dev, > > struct resource *tco_res) > > { > > + static DEFINE_MUTEX(p2sb_mutex); > > To be on the safe side, we should explicitly #include <linux/mutex.h>. > > > struct resource *res; > > unsigned int devfn; > > u64 base64_addr; > > @@ -1510,7 +1509,7 @@ i801_add_tco_spt(struct i801_priv *priv, struct pci_dev *pci_dev, > > * enumerated by the PCI subsystem, so we need to unhide/hide it > > * to lookup the P2SB BAR. > > */ > > - spin_lock(&p2sb_spinlock); > > + mutex_lock(&p2sb_mutex); > > > > devfn = PCI_DEVFN(PCI_SLOT(pci_dev->devfn), 1); > > > > @@ -1528,7 +1527,7 @@ i801_add_tco_spt(struct i801_priv *priv, struct pci_dev *pci_dev, > > /* Hide the P2SB device, if it was hidden before */ > > if (hidden) > > pci_bus_write_config_byte(pci_dev->bus, devfn, 0xe1, hidden); > > - spin_unlock(&p2sb_spinlock); > > + mutex_unlock(&p2sb_mutex); > > > > res = &tco_res[1]; > > if (pci_dev->device == PCI_DEVICE_ID_INTEL_DNV_SMBUS) > > Reviewed-by: Jean Delvare <jdelvare@suse.de> > > -- > Jean Delvare > SUSE L3 Support
diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c index bdb619bc0..504f02e1e 100644 --- a/drivers/i2c/busses/i2c-i801.c +++ b/drivers/i2c/busses/i2c-i801.c @@ -1492,12 +1492,11 @@ static const struct itco_wdt_platform_data spt_tco_platform_data = { .version = 4, }; -static DEFINE_SPINLOCK(p2sb_spinlock); - static struct platform_device * i801_add_tco_spt(struct i801_priv *priv, struct pci_dev *pci_dev, struct resource *tco_res) { + static DEFINE_MUTEX(p2sb_mutex); struct resource *res; unsigned int devfn; u64 base64_addr; @@ -1510,7 +1509,7 @@ i801_add_tco_spt(struct i801_priv *priv, struct pci_dev *pci_dev, * enumerated by the PCI subsystem, so we need to unhide/hide it * to lookup the P2SB BAR. */ - spin_lock(&p2sb_spinlock); + mutex_lock(&p2sb_mutex); devfn = PCI_DEVFN(PCI_SLOT(pci_dev->devfn), 1); @@ -1528,7 +1527,7 @@ i801_add_tco_spt(struct i801_priv *priv, struct pci_dev *pci_dev, /* Hide the P2SB device, if it was hidden before */ if (hidden) pci_bus_write_config_byte(pci_dev->bus, devfn, 0xe1, hidden); - spin_unlock(&p2sb_spinlock); + mutex_unlock(&p2sb_mutex); res = &tco_res[1]; if (pci_dev->device == PCI_DEVICE_ID_INTEL_DNV_SMBUS)
p2sb_spinlock is used in i801_add_tco_spt() only and in process context only. Therefore a mutex is sufficient, and we can make the definition local to i801_add_tco_spt(). Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> --- drivers/i2c/busses/i2c-i801.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)