* [PATCH v1] driver: gpio-bt8xx: use generic PCI PM @ 2025-10-10 10:53 Vaibhav Gupta 2025-10-11 1:43 ` kernel test robot 0 siblings, 1 reply; 23+ messages in thread From: Vaibhav Gupta @ 2025-10-10 10:53 UTC (permalink / raw) To: Michael Buesch Cc: Vaibhav Gupta, Bjorn Helgaas, Linus Walleij, Bartosz Golaszewski, linux-gpio, linux-kernel Switch to the new generic PCI power management framework and remove legacy callbacks like .suspend() and .resume(). With the generic framework, the standard PCI related work like: - pci_save/restore_state() - pci_enable/disable_device() - pci_set_power_state() is handled by the PCI core and this driver should implement only gpio-bt8xx specific operations in its respective callback functions. Signed-off-by: Vaibhav Gupta <vaibhavgupta40@gmail.com> --- drivers/gpio/gpio-bt8xx.c | 27 +++++++-------------------- 1 file changed, 7 insertions(+), 20 deletions(-) diff --git a/drivers/gpio/gpio-bt8xx.c b/drivers/gpio/gpio-bt8xx.c index 05401da03ca3..e8d0c67bb618 100644 --- a/drivers/gpio/gpio-bt8xx.c +++ b/drivers/gpio/gpio-bt8xx.c @@ -224,9 +224,9 @@ static void bt8xxgpio_remove(struct pci_dev *pdev) pci_disable_device(pdev); } -#ifdef CONFIG_PM -static int bt8xxgpio_suspend(struct pci_dev *pdev, pm_message_t state) +static int __maybe_unused bt8xxgpio_suspend(struct device *dev) { + struct pci_dev *pdev = to_pci_dev(dev); struct bt8xxgpio *bg = pci_get_drvdata(pdev); scoped_guard(spinlock_irqsave, &bg->lock) { @@ -238,23 +238,13 @@ static int bt8xxgpio_suspend(struct pci_dev *pdev, pm_message_t state) bgwrite(0x0, BT848_GPIO_OUT_EN); } - pci_save_state(pdev); - pci_disable_device(pdev); - pci_set_power_state(pdev, pci_choose_state(pdev, state)); - return 0; } -static int bt8xxgpio_resume(struct pci_dev *pdev) +static int __maybe_unused bt8xxgpio_resume(struct device *dev) { + struct pci_dev *pdev = to_pci_dev(dev); struct bt8xxgpio *bg = pci_get_drvdata(pdev); - int err; - - pci_set_power_state(pdev, PCI_D0); - err = pci_enable_device(pdev); - if (err) - return err; - pci_restore_state(pdev); guard(spinlock_irqsave)(&bg->lock); @@ -267,10 +257,8 @@ static int bt8xxgpio_resume(struct pci_dev *pdev) return 0; } -#else -#define bt8xxgpio_suspend NULL -#define bt8xxgpio_resume NULL -#endif /* CONFIG_PM */ + +static SIMPLE_DEV_PM_OPS(bt8xxgpio_pm_ops, bt8xxgpio_suspend, bt8xxgpio_resume); static const struct pci_device_id bt8xxgpio_pci_tbl[] = { { PCI_DEVICE(PCI_VENDOR_ID_BROOKTREE, PCI_DEVICE_ID_BT848) }, @@ -286,8 +274,7 @@ static struct pci_driver bt8xxgpio_pci_driver = { .id_table = bt8xxgpio_pci_tbl, .probe = bt8xxgpio_probe, .remove = bt8xxgpio_remove, - .suspend = bt8xxgpio_suspend, - .resume = bt8xxgpio_resume, + .driver.pm = &bt8xxgpio_pm_ops, }; module_pci_driver(bt8xxgpio_pci_driver); -- 2.51.0 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH v1] driver: gpio-bt8xx: use generic PCI PM 2025-10-10 10:53 [PATCH v1] driver: gpio-bt8xx: use generic PCI PM Vaibhav Gupta @ 2025-10-11 1:43 ` kernel test robot 2025-10-11 10:26 ` Michael Büsch 0 siblings, 1 reply; 23+ messages in thread From: kernel test robot @ 2025-10-11 1:43 UTC (permalink / raw) To: Vaibhav Gupta, Michael Buesch Cc: oe-kbuild-all, Vaibhav Gupta, Bjorn Helgaas, Linus Walleij, Bartosz Golaszewski, linux-gpio, linux-kernel Hi Vaibhav, kernel test robot noticed the following build errors: [auto build test ERROR on brgl/gpio/for-next] [also build test ERROR on linus/master v6.17 next-20251010] [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/Vaibhav-Gupta/driver-gpio-bt8xx-use-generic-PCI-PM/20251010-185625 base: https://git.kernel.org/pub/scm/linux/kernel/git/brgl/linux.git gpio/for-next patch link: https://lore.kernel.org/r/20251010105338.664564-1-vaibhavgupta40%40gmail.com patch subject: [PATCH v1] driver: gpio-bt8xx: use generic PCI PM config: loongarch-randconfig-002-20251011 (https://download.01.org/0day-ci/archive/20251011/202510110924.dUQeeRV6-lkp@intel.com/config) compiler: loongarch64-linux-gcc (GCC) 12.5.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251011/202510110924.dUQeeRV6-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202510110924.dUQeeRV6-lkp@intel.com/ All errors (new ones prefixed by >>): drivers/gpio/gpio-bt8xx.c: In function 'bt8xxgpio_suspend': >> drivers/gpio/gpio-bt8xx.c:233:19: error: 'struct bt8xxgpio' has no member named 'saved_outen' 233 | bg->saved_outen = bgread(BT848_GPIO_OUT_EN); | ^~ >> drivers/gpio/gpio-bt8xx.c:234:19: error: 'struct bt8xxgpio' has no member named 'saved_data' 234 | bg->saved_data = bgread(BT848_GPIO_DATA); | ^~ drivers/gpio/gpio-bt8xx.c: In function 'bt8xxgpio_resume': drivers/gpio/gpio-bt8xx.c:254:19: error: 'struct bt8xxgpio' has no member named 'saved_outen' 254 | bgwrite(bg->saved_outen, BT848_GPIO_OUT_EN); | ^~ drivers/gpio/gpio-bt8xx.c:61:41: note: in definition of macro 'bgwrite' 61 | #define bgwrite(dat, adr) writel((dat), bg->mmio+(adr)) | ^~~ drivers/gpio/gpio-bt8xx.c:255:19: error: 'struct bt8xxgpio' has no member named 'saved_data' 255 | bgwrite(bg->saved_data & bg->saved_outen, | ^~ drivers/gpio/gpio-bt8xx.c:61:41: note: in definition of macro 'bgwrite' 61 | #define bgwrite(dat, adr) writel((dat), bg->mmio+(adr)) | ^~~ drivers/gpio/gpio-bt8xx.c:255:36: error: 'struct bt8xxgpio' has no member named 'saved_outen' 255 | bgwrite(bg->saved_data & bg->saved_outen, | ^~ drivers/gpio/gpio-bt8xx.c:61:41: note: in definition of macro 'bgwrite' 61 | #define bgwrite(dat, adr) writel((dat), bg->mmio+(adr)) | ^~~ vim +233 drivers/gpio/gpio-bt8xx.c ff1d5c2f0268f4 drivers/gpio/bt8xxgpio.c Michael Buesch 2008-07-25 226 2213c7a2cb81b2 drivers/gpio/gpio-bt8xx.c Vaibhav Gupta 2025-10-10 227 static int __maybe_unused bt8xxgpio_suspend(struct device *dev) ff1d5c2f0268f4 drivers/gpio/bt8xxgpio.c Michael Buesch 2008-07-25 228 { 2213c7a2cb81b2 drivers/gpio/gpio-bt8xx.c Vaibhav Gupta 2025-10-10 229 struct pci_dev *pdev = to_pci_dev(dev); ff1d5c2f0268f4 drivers/gpio/bt8xxgpio.c Michael Buesch 2008-07-25 230 struct bt8xxgpio *bg = pci_get_drvdata(pdev); ff1d5c2f0268f4 drivers/gpio/bt8xxgpio.c Michael Buesch 2008-07-25 231 b9a557d05a7dde drivers/gpio/gpio-bt8xx.c Bartosz Golaszewski 2025-03-10 232 scoped_guard(spinlock_irqsave, &bg->lock) { ff1d5c2f0268f4 drivers/gpio/bt8xxgpio.c Michael Buesch 2008-07-25 @233 bg->saved_outen = bgread(BT848_GPIO_OUT_EN); ff1d5c2f0268f4 drivers/gpio/bt8xxgpio.c Michael Buesch 2008-07-25 @234 bg->saved_data = bgread(BT848_GPIO_DATA); ff1d5c2f0268f4 drivers/gpio/bt8xxgpio.c Michael Buesch 2008-07-25 235 ff1d5c2f0268f4 drivers/gpio/bt8xxgpio.c Michael Buesch 2008-07-25 236 bgwrite(0, BT848_INT_MASK); ff1d5c2f0268f4 drivers/gpio/bt8xxgpio.c Michael Buesch 2008-07-25 237 bgwrite(~0x0, BT848_INT_STAT); ff1d5c2f0268f4 drivers/gpio/bt8xxgpio.c Michael Buesch 2008-07-25 238 bgwrite(0x0, BT848_GPIO_OUT_EN); b9a557d05a7dde drivers/gpio/gpio-bt8xx.c Bartosz Golaszewski 2025-03-10 239 } ff1d5c2f0268f4 drivers/gpio/bt8xxgpio.c Michael Buesch 2008-07-25 240 ff1d5c2f0268f4 drivers/gpio/bt8xxgpio.c Michael Buesch 2008-07-25 241 return 0; ff1d5c2f0268f4 drivers/gpio/bt8xxgpio.c Michael Buesch 2008-07-25 242 } ff1d5c2f0268f4 drivers/gpio/bt8xxgpio.c Michael Buesch 2008-07-25 243 -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v1] driver: gpio-bt8xx: use generic PCI PM 2025-10-11 1:43 ` kernel test robot @ 2025-10-11 10:26 ` Michael Büsch 2025-10-11 11:32 ` Vaibhav Gupta 0 siblings, 1 reply; 23+ messages in thread From: Michael Büsch @ 2025-10-11 10:26 UTC (permalink / raw) To: Vaibhav Gupta Cc: kernel test robot, oe-kbuild-all, Bjorn Helgaas, Linus Walleij, Bartosz Golaszewski, linux-gpio, linux-kernel [-- Attachment #1: Type: text/plain, Size: 569 bytes --] On Sat, 11 Oct 2025 09:43:54 +0800 kernel test robot <lkp@intel.com> wrote: > Hi Vaibhav, > > kernel test robot noticed the following build errors: > drivers/gpio/gpio-bt8xx.c: In function 'bt8xxgpio_suspend': > >> drivers/gpio/gpio-bt8xx.c:233:19: error: 'struct bt8xxgpio' has no member named 'saved_outen' > 233 | bg->saved_outen = bgread(BT848_GPIO_OUT_EN); > | ^~ It looks like the #ifdef CONFIG_PM must be removed from struct bt8xxgpio definition. -- Michael Büsch https://bues.ch/ [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v1] driver: gpio-bt8xx: use generic PCI PM 2025-10-11 10:26 ` Michael Büsch @ 2025-10-11 11:32 ` Vaibhav Gupta 2025-10-11 12:31 ` Michael Büsch ` (2 more replies) 0 siblings, 3 replies; 23+ messages in thread From: Vaibhav Gupta @ 2025-10-11 11:32 UTC (permalink / raw) To: Michael Büsch Cc: kernel test robot, oe-kbuild-all, Bjorn Helgaas, Linus Walleij, Bartosz Golaszewski, linux-gpio, linux-kernel On Sat, Oct 11, 2025 at 12:26:12PM +0200, Michael Büsch wrote: > On Sat, 11 Oct 2025 09:43:54 +0800 > kernel test robot <lkp@intel.com> wrote: > > > Hi Vaibhav, > > > > kernel test robot noticed the following build errors: > > > drivers/gpio/gpio-bt8xx.c: In function 'bt8xxgpio_suspend': > > >> drivers/gpio/gpio-bt8xx.c:233:19: error: 'struct bt8xxgpio' has no member named 'saved_outen' > > 233 | bg->saved_outen = bgread(BT848_GPIO_OUT_EN); > > | ^~ > > > It looks like the > #ifdef CONFIG_PM > must be removed from struct bt8xxgpio definition. > > -- > Michael Büsch > https://bues.ch/ Hello Michael, Ah yes, this macro somehow got overlooked by me. I will send a v2. Thanks for the review! Regards, Vaibhav ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v1] driver: gpio-bt8xx: use generic PCI PM 2025-10-11 11:32 ` Vaibhav Gupta @ 2025-10-11 12:31 ` Michael Büsch 2025-10-11 12:37 ` Vaibhav Gupta 2025-10-11 12:57 ` [PATCH v3] " Vaibhav Gupta 2025-10-11 12:32 ` [PATCH v2] driver: gpio-bt8xx: " Vaibhav Gupta 2025-10-13 9:41 ` [PATCH v1] " Bartosz Golaszewski 2 siblings, 2 replies; 23+ messages in thread From: Michael Büsch @ 2025-10-11 12:31 UTC (permalink / raw) To: Vaibhav Gupta Cc: kernel test robot, oe-kbuild-all, Bjorn Helgaas, Linus Walleij, Bartosz Golaszewski, linux-gpio, linux-kernel [-- Attachment #1: Type: text/plain, Size: 487 bytes --] On Sat, 11 Oct 2025 11:32:11 +0000 Vaibhav Gupta <vaibhavgupta40@gmail.com> wrote: > > It looks like the > > #ifdef CONFIG_PM > > must be removed from struct bt8xxgpio definition. > Ah yes, this macro somehow got overlooked by me. I will send a v2. > Thanks for the review! Also, the SIMPLE_DEV_PM_OPS macro seems to be deprecated in favour of DEFINE_SIMPLE_DEV_PM_OPS. And please do a compile test when submitting v2. Thanks :) -- Michael Büsch https://bues.ch/ [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v1] driver: gpio-bt8xx: use generic PCI PM 2025-10-11 12:31 ` Michael Büsch @ 2025-10-11 12:37 ` Vaibhav Gupta 2025-10-11 12:57 ` [PATCH v3] " Vaibhav Gupta 1 sibling, 0 replies; 23+ messages in thread From: Vaibhav Gupta @ 2025-10-11 12:37 UTC (permalink / raw) To: Michael Büsch Cc: kernel test robot, oe-kbuild-all, Bjorn Helgaas, Linus Walleij, Bartosz Golaszewski, linux-gpio, linux-kernel On Sat, Oct 11, 2025 at 02:31:23PM +0200, Michael Büsch wrote: > On Sat, 11 Oct 2025 11:32:11 +0000 > Vaibhav Gupta <vaibhavgupta40@gmail.com> wrote: > > > > It looks like the > > > #ifdef CONFIG_PM > > > must be removed from struct bt8xxgpio definition. > > Ah yes, this macro somehow got overlooked by me. I will send a v2. > > Thanks for the review! > > > Also, the SIMPLE_DEV_PM_OPS macro seems to be deprecated in favour of DEFINE_SIMPLE_DEV_PM_OPS. Okay I will have a look to it. Ingonre v2 in this case. > > And please do a compile test when submitting v2. Thanks :) Sure! I created another config file to disable CONFIG_PM just to make sure during compile test. Regards, Vaibhav > > -- > Michael Büsch > https://bues.ch/ ^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v3] driver: gpio-bt8xx: use generic PCI PM 2025-10-11 12:31 ` Michael Büsch 2025-10-11 12:37 ` Vaibhav Gupta @ 2025-10-11 12:57 ` Vaibhav Gupta 2025-10-13 11:04 ` [PATCH v4] driver: gpio: bt8xx: " Vaibhav Gupta 1 sibling, 1 reply; 23+ messages in thread From: Vaibhav Gupta @ 2025-10-11 12:57 UTC (permalink / raw) To: Michael Buesch Cc: Vaibhav Gupta, Bjorn Helgaas, Linus Walleij, Bartosz Golaszewski, linux-gpio, linux-kernel Switch to the new generic PCI power management framework and remove legacy callbacks like .suspend() and .resume(). With the generic framework, the standard PCI related work like: - pci_save/restore_state() - pci_enable/disable_device() - pci_set_power_state() is handled by the PCI core and this driver should implement only gpio-bt8xx specific operations in its respective callback functions. Signed-off-by: Vaibhav Gupta <vaibhavgupta40@gmail.com> --- drivers/gpio/gpio-bt8xx.c | 29 +++++++---------------------- 1 file changed, 7 insertions(+), 22 deletions(-) diff --git a/drivers/gpio/gpio-bt8xx.c b/drivers/gpio/gpio-bt8xx.c index 05401da03ca3..70b49c385b43 100644 --- a/drivers/gpio/gpio-bt8xx.c +++ b/drivers/gpio/gpio-bt8xx.c @@ -52,10 +52,8 @@ struct bt8xxgpio { struct pci_dev *pdev; struct gpio_chip gpio; -#ifdef CONFIG_PM u32 saved_outen; u32 saved_data; -#endif }; #define bgwrite(dat, adr) writel((dat), bg->mmio+(adr)) @@ -224,9 +222,9 @@ static void bt8xxgpio_remove(struct pci_dev *pdev) pci_disable_device(pdev); } -#ifdef CONFIG_PM -static int bt8xxgpio_suspend(struct pci_dev *pdev, pm_message_t state) +static int __maybe_unused bt8xxgpio_suspend(struct device *dev) { + struct pci_dev *pdev = to_pci_dev(dev); struct bt8xxgpio *bg = pci_get_drvdata(pdev); scoped_guard(spinlock_irqsave, &bg->lock) { @@ -238,23 +236,13 @@ static int bt8xxgpio_suspend(struct pci_dev *pdev, pm_message_t state) bgwrite(0x0, BT848_GPIO_OUT_EN); } - pci_save_state(pdev); - pci_disable_device(pdev); - pci_set_power_state(pdev, pci_choose_state(pdev, state)); - return 0; } -static int bt8xxgpio_resume(struct pci_dev *pdev) +static int __maybe_unused bt8xxgpio_resume(struct device *dev) { + struct pci_dev *pdev = to_pci_dev(dev); struct bt8xxgpio *bg = pci_get_drvdata(pdev); - int err; - - pci_set_power_state(pdev, PCI_D0); - err = pci_enable_device(pdev); - if (err) - return err; - pci_restore_state(pdev); guard(spinlock_irqsave)(&bg->lock); @@ -267,10 +255,8 @@ static int bt8xxgpio_resume(struct pci_dev *pdev) return 0; } -#else -#define bt8xxgpio_suspend NULL -#define bt8xxgpio_resume NULL -#endif /* CONFIG_PM */ + +static DEFINE_SIMPLE_DEV_PM_OPS(bt8xxgpio_pm_ops, bt8xxgpio_suspend, bt8xxgpio_resume); static const struct pci_device_id bt8xxgpio_pci_tbl[] = { { PCI_DEVICE(PCI_VENDOR_ID_BROOKTREE, PCI_DEVICE_ID_BT848) }, @@ -286,8 +272,7 @@ static struct pci_driver bt8xxgpio_pci_driver = { .id_table = bt8xxgpio_pci_tbl, .probe = bt8xxgpio_probe, .remove = bt8xxgpio_remove, - .suspend = bt8xxgpio_suspend, - .resume = bt8xxgpio_resume, + .driver.pm = &bt8xxgpio_pm_ops, }; module_pci_driver(bt8xxgpio_pci_driver); -- 2.51.0 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v4] driver: gpio: bt8xx: use generic PCI PM 2025-10-11 12:57 ` [PATCH v3] " Vaibhav Gupta @ 2025-10-13 11:04 ` Vaibhav Gupta 0 siblings, 0 replies; 23+ messages in thread From: Vaibhav Gupta @ 2025-10-13 11:04 UTC (permalink / raw) To: Michael Buesch Cc: Vaibhav Gupta, Bjorn Helgaas, Linus Walleij, Bartosz Golaszewski, linux-gpio, linux-kernel Switch to the new generic PCI power management framework and remove legacy callbacks like .suspend() and .resume(). With the generic framework, the standard PCI related work like: - pci_save/restore_state() - pci_enable/disable_device() - pci_set_power_state() is handled by the PCI core and this driver should implement only gpio-bt8xx specific operations in its respective callback functions. Signed-off-by: Vaibhav Gupta <vaibhavgupta40@gmail.com> --- drivers/gpio/gpio-bt8xx.c | 29 +++++++---------------------- 1 file changed, 7 insertions(+), 22 deletions(-) diff --git a/drivers/gpio/gpio-bt8xx.c b/drivers/gpio/gpio-bt8xx.c index 05401da03ca3..70b49c385b43 100644 --- a/drivers/gpio/gpio-bt8xx.c +++ b/drivers/gpio/gpio-bt8xx.c @@ -52,10 +52,8 @@ struct bt8xxgpio { struct pci_dev *pdev; struct gpio_chip gpio; -#ifdef CONFIG_PM u32 saved_outen; u32 saved_data; -#endif }; #define bgwrite(dat, adr) writel((dat), bg->mmio+(adr)) @@ -224,9 +222,9 @@ static void bt8xxgpio_remove(struct pci_dev *pdev) pci_disable_device(pdev); } -#ifdef CONFIG_PM -static int bt8xxgpio_suspend(struct pci_dev *pdev, pm_message_t state) +static int __maybe_unused bt8xxgpio_suspend(struct device *dev) { + struct pci_dev *pdev = to_pci_dev(dev); struct bt8xxgpio *bg = pci_get_drvdata(pdev); scoped_guard(spinlock_irqsave, &bg->lock) { @@ -238,23 +236,13 @@ static int bt8xxgpio_suspend(struct pci_dev *pdev, pm_message_t state) bgwrite(0x0, BT848_GPIO_OUT_EN); } - pci_save_state(pdev); - pci_disable_device(pdev); - pci_set_power_state(pdev, pci_choose_state(pdev, state)); - return 0; } -static int bt8xxgpio_resume(struct pci_dev *pdev) +static int __maybe_unused bt8xxgpio_resume(struct device *dev) { + struct pci_dev *pdev = to_pci_dev(dev); struct bt8xxgpio *bg = pci_get_drvdata(pdev); - int err; - - pci_set_power_state(pdev, PCI_D0); - err = pci_enable_device(pdev); - if (err) - return err; - pci_restore_state(pdev); guard(spinlock_irqsave)(&bg->lock); @@ -267,10 +255,8 @@ static int bt8xxgpio_resume(struct pci_dev *pdev) return 0; } -#else -#define bt8xxgpio_suspend NULL -#define bt8xxgpio_resume NULL -#endif /* CONFIG_PM */ + +static DEFINE_SIMPLE_DEV_PM_OPS(bt8xxgpio_pm_ops, bt8xxgpio_suspend, bt8xxgpio_resume); static const struct pci_device_id bt8xxgpio_pci_tbl[] = { { PCI_DEVICE(PCI_VENDOR_ID_BROOKTREE, PCI_DEVICE_ID_BT848) }, @@ -286,8 +272,7 @@ static struct pci_driver bt8xxgpio_pci_driver = { .id_table = bt8xxgpio_pci_tbl, .probe = bt8xxgpio_probe, .remove = bt8xxgpio_remove, - .suspend = bt8xxgpio_suspend, - .resume = bt8xxgpio_resume, + .driver.pm = &bt8xxgpio_pm_ops, }; module_pci_driver(bt8xxgpio_pci_driver); -- 2.51.0 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v2] driver: gpio-bt8xx: use generic PCI PM 2025-10-11 11:32 ` Vaibhav Gupta 2025-10-11 12:31 ` Michael Büsch @ 2025-10-11 12:32 ` Vaibhav Gupta 2025-10-13 9:41 ` [PATCH v1] " Bartosz Golaszewski 2 siblings, 0 replies; 23+ messages in thread From: Vaibhav Gupta @ 2025-10-11 12:32 UTC (permalink / raw) To: Michael Buesch Cc: Vaibhav Gupta, Bjorn Helgaas, Linus Walleij, Bartosz Golaszewski, linux-gpio, linux-kernel Switch to the new generic PCI power management framework and remove legacy callbacks like .suspend() and .resume(). With the generic framework, the standard PCI related work like: - pci_save/restore_state() - pci_enable/disable_device() - pci_set_power_state() is handled by the PCI core and this driver should implement only gpio-bt8xx specific operations in its respective callback functions. Signed-off-by: Vaibhav Gupta <vaibhavgupta40@gmail.com> --- drivers/gpio/gpio-bt8xx.c | 29 +++++++---------------------- 1 file changed, 7 insertions(+), 22 deletions(-) diff --git a/drivers/gpio/gpio-bt8xx.c b/drivers/gpio/gpio-bt8xx.c index 05401da03ca3..451ec38c350d 100644 --- a/drivers/gpio/gpio-bt8xx.c +++ b/drivers/gpio/gpio-bt8xx.c @@ -52,10 +52,8 @@ struct bt8xxgpio { struct pci_dev *pdev; struct gpio_chip gpio; -#ifdef CONFIG_PM u32 saved_outen; u32 saved_data; -#endif }; #define bgwrite(dat, adr) writel((dat), bg->mmio+(adr)) @@ -224,9 +222,9 @@ static void bt8xxgpio_remove(struct pci_dev *pdev) pci_disable_device(pdev); } -#ifdef CONFIG_PM -static int bt8xxgpio_suspend(struct pci_dev *pdev, pm_message_t state) +static int __maybe_unused bt8xxgpio_suspend(struct device *dev) { + struct pci_dev *pdev = to_pci_dev(dev); struct bt8xxgpio *bg = pci_get_drvdata(pdev); scoped_guard(spinlock_irqsave, &bg->lock) { @@ -238,23 +236,13 @@ static int bt8xxgpio_suspend(struct pci_dev *pdev, pm_message_t state) bgwrite(0x0, BT848_GPIO_OUT_EN); } - pci_save_state(pdev); - pci_disable_device(pdev); - pci_set_power_state(pdev, pci_choose_state(pdev, state)); - return 0; } -static int bt8xxgpio_resume(struct pci_dev *pdev) +static int __maybe_unused bt8xxgpio_resume(struct device *dev) { + struct pci_dev *pdev = to_pci_dev(dev); struct bt8xxgpio *bg = pci_get_drvdata(pdev); - int err; - - pci_set_power_state(pdev, PCI_D0); - err = pci_enable_device(pdev); - if (err) - return err; - pci_restore_state(pdev); guard(spinlock_irqsave)(&bg->lock); @@ -267,10 +255,8 @@ static int bt8xxgpio_resume(struct pci_dev *pdev) return 0; } -#else -#define bt8xxgpio_suspend NULL -#define bt8xxgpio_resume NULL -#endif /* CONFIG_PM */ + +static SIMPLE_DEV_PM_OPS(bt8xxgpio_pm_ops, bt8xxgpio_suspend, bt8xxgpio_resume); static const struct pci_device_id bt8xxgpio_pci_tbl[] = { { PCI_DEVICE(PCI_VENDOR_ID_BROOKTREE, PCI_DEVICE_ID_BT848) }, @@ -286,8 +272,7 @@ static struct pci_driver bt8xxgpio_pci_driver = { .id_table = bt8xxgpio_pci_tbl, .probe = bt8xxgpio_probe, .remove = bt8xxgpio_remove, - .suspend = bt8xxgpio_suspend, - .resume = bt8xxgpio_resume, + .driver.pm = &bt8xxgpio_pm_ops, }; module_pci_driver(bt8xxgpio_pci_driver); -- 2.51.0 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH v1] driver: gpio-bt8xx: use generic PCI PM 2025-10-11 11:32 ` Vaibhav Gupta 2025-10-11 12:31 ` Michael Büsch 2025-10-11 12:32 ` [PATCH v2] driver: gpio-bt8xx: " Vaibhav Gupta @ 2025-10-13 9:41 ` Bartosz Golaszewski 2025-10-13 11:03 ` Vaibhav Gupta 2 siblings, 1 reply; 23+ messages in thread From: Bartosz Golaszewski @ 2025-10-13 9:41 UTC (permalink / raw) To: Vaibhav Gupta Cc: Michael Büsch, kernel test robot, oe-kbuild-all, Bjorn Helgaas, Linus Walleij, linux-gpio, linux-kernel On Sat, Oct 11, 2025 at 1:32 PM Vaibhav Gupta <vaibhavgupta40@gmail.com> wrote: > > On Sat, Oct 11, 2025 at 12:26:12PM +0200, Michael Büsch wrote: > > On Sat, 11 Oct 2025 09:43:54 +0800 > > kernel test robot <lkp@intel.com> wrote: > > > > > Hi Vaibhav, > > > > > > kernel test robot noticed the following build errors: > > > > > drivers/gpio/gpio-bt8xx.c: In function 'bt8xxgpio_suspend': > > > >> drivers/gpio/gpio-bt8xx.c:233:19: error: 'struct bt8xxgpio' has no member named 'saved_outen' > > > 233 | bg->saved_outen = bgread(BT848_GPIO_OUT_EN); > > > | ^~ > > > > > > It looks like the > > #ifdef CONFIG_PM > > must be removed from struct bt8xxgpio definition. > > > > -- > > Michael Büsch > > https://bues.ch/ > > Hello Michael, > > Ah yes, this macro somehow got overlooked by me. I will send a v2. > Thanks for the review! > > Regards, > Vaibhav While at it: the subject should be: "gpio: bt8xx: ..." Bart ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v1] driver: gpio-bt8xx: use generic PCI PM 2025-10-13 9:41 ` [PATCH v1] " Bartosz Golaszewski @ 2025-10-13 11:03 ` Vaibhav Gupta 2025-10-13 15:36 ` Bartosz Golaszewski 0 siblings, 1 reply; 23+ messages in thread From: Vaibhav Gupta @ 2025-10-13 11:03 UTC (permalink / raw) To: Bartosz Golaszewski Cc: Michael Büsch, kernel test robot, oe-kbuild-all, Bjorn Helgaas, Linus Walleij, linux-gpio, linux-kernel On Mon, Oct 13, 2025 at 11:41:43AM +0200, Bartosz Golaszewski wrote: > On Sat, Oct 11, 2025 at 1:32 PM Vaibhav Gupta <vaibhavgupta40@gmail.com> wrote: > > > > > > Hello Michael, > > > > Ah yes, this macro somehow got overlooked by me. I will send a v2. > > Thanks for the review! > > > > Regards, > > Vaibhav > > While at it: the subject should be: "gpio: bt8xx: ..." > > Bart Done in v4. Vaibhav ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v1] driver: gpio-bt8xx: use generic PCI PM 2025-10-13 11:03 ` Vaibhav Gupta @ 2025-10-13 15:36 ` Bartosz Golaszewski 2025-10-13 15:51 ` Vaibhav Gupta 0 siblings, 1 reply; 23+ messages in thread From: Bartosz Golaszewski @ 2025-10-13 15:36 UTC (permalink / raw) To: Vaibhav Gupta Cc: Michael Büsch, kernel test robot, oe-kbuild-all, Bjorn Helgaas, Linus Walleij, linux-gpio, linux-kernel On Mon, Oct 13, 2025 at 1:03 PM Vaibhav Gupta <vaibhavgupta40@gmail.com> wrote: > > On Mon, Oct 13, 2025 at 11:41:43AM +0200, Bartosz Golaszewski wrote: > > On Sat, Oct 11, 2025 at 1:32 PM Vaibhav Gupta <vaibhavgupta40@gmail.com> wrote: > > > > > > > > > Hello Michael, > > > > > > Ah yes, this macro somehow got overlooked by me. I will send a v2. > > > Thanks for the review! > > > > > > Regards, > > > Vaibhav > > > > While at it: the subject should be: "gpio: bt8xx: ..." > > > > Bart > > Done in v4. > > Vaibhav No, it was not. Bartosz ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v1] driver: gpio-bt8xx: use generic PCI PM 2025-10-13 15:36 ` Bartosz Golaszewski @ 2025-10-13 15:51 ` Vaibhav Gupta 2025-10-13 15:52 ` [PATCH v5] gpio: bt8xx: " Vaibhav Gupta 0 siblings, 1 reply; 23+ messages in thread From: Vaibhav Gupta @ 2025-10-13 15:51 UTC (permalink / raw) To: Bartosz Golaszewski Cc: Michael Büsch, kernel test robot, oe-kbuild-all, Bjorn Helgaas, Linus Walleij, linux-gpio, linux-kernel On Mon, Oct 13, 2025 at 05:36:17PM +0200, Bartosz Golaszewski wrote: > On Mon, Oct 13, 2025 at 1:03 PM Vaibhav Gupta <vaibhavgupta40@gmail.com> wrote: > > > > > > > > While at it: the subject should be: "gpio: bt8xx: ..." > > > > > > Bart > > > > Done in v4. > > > > Vaibhav > > No, it was not. > > Bartosz Ah I see, I though you just wanted the transformation of "gpio-bt8xx:" to "gpio: bt8xx:". I've removed "driver:" as well in v5 from the header. Vaibhav ^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v5] gpio: bt8xx: use generic PCI PM 2025-10-13 15:51 ` Vaibhav Gupta @ 2025-10-13 15:52 ` Vaibhav Gupta 2025-10-13 17:43 ` Bjorn Helgaas 0 siblings, 1 reply; 23+ messages in thread From: Vaibhav Gupta @ 2025-10-13 15:52 UTC (permalink / raw) To: Michael Buesch, Bartosz Golaszewski Cc: Vaibhav Gupta, Bjorn Helgaas, Linus Walleij, linux-gpio, linux-kernel Switch to the new generic PCI power management framework and remove legacy callbacks like .suspend() and .resume(). With the generic framework, the standard PCI related work like: - pci_save/restore_state() - pci_enable/disable_device() - pci_set_power_state() is handled by the PCI core and this driver should implement only gpio-bt8xx specific operations in its respective callback functions. Signed-off-by: Vaibhav Gupta <vaibhavgupta40@gmail.com> --- drivers/gpio/gpio-bt8xx.c | 29 +++++++---------------------- 1 file changed, 7 insertions(+), 22 deletions(-) diff --git a/drivers/gpio/gpio-bt8xx.c b/drivers/gpio/gpio-bt8xx.c index 05401da03ca3..70b49c385b43 100644 --- a/drivers/gpio/gpio-bt8xx.c +++ b/drivers/gpio/gpio-bt8xx.c @@ -52,10 +52,8 @@ struct bt8xxgpio { struct pci_dev *pdev; struct gpio_chip gpio; -#ifdef CONFIG_PM u32 saved_outen; u32 saved_data; -#endif }; #define bgwrite(dat, adr) writel((dat), bg->mmio+(adr)) @@ -224,9 +222,9 @@ static void bt8xxgpio_remove(struct pci_dev *pdev) pci_disable_device(pdev); } -#ifdef CONFIG_PM -static int bt8xxgpio_suspend(struct pci_dev *pdev, pm_message_t state) +static int __maybe_unused bt8xxgpio_suspend(struct device *dev) { + struct pci_dev *pdev = to_pci_dev(dev); struct bt8xxgpio *bg = pci_get_drvdata(pdev); scoped_guard(spinlock_irqsave, &bg->lock) { @@ -238,23 +236,13 @@ static int bt8xxgpio_suspend(struct pci_dev *pdev, pm_message_t state) bgwrite(0x0, BT848_GPIO_OUT_EN); } - pci_save_state(pdev); - pci_disable_device(pdev); - pci_set_power_state(pdev, pci_choose_state(pdev, state)); - return 0; } -static int bt8xxgpio_resume(struct pci_dev *pdev) +static int __maybe_unused bt8xxgpio_resume(struct device *dev) { + struct pci_dev *pdev = to_pci_dev(dev); struct bt8xxgpio *bg = pci_get_drvdata(pdev); - int err; - - pci_set_power_state(pdev, PCI_D0); - err = pci_enable_device(pdev); - if (err) - return err; - pci_restore_state(pdev); guard(spinlock_irqsave)(&bg->lock); @@ -267,10 +255,8 @@ static int bt8xxgpio_resume(struct pci_dev *pdev) return 0; } -#else -#define bt8xxgpio_suspend NULL -#define bt8xxgpio_resume NULL -#endif /* CONFIG_PM */ + +static DEFINE_SIMPLE_DEV_PM_OPS(bt8xxgpio_pm_ops, bt8xxgpio_suspend, bt8xxgpio_resume); static const struct pci_device_id bt8xxgpio_pci_tbl[] = { { PCI_DEVICE(PCI_VENDOR_ID_BROOKTREE, PCI_DEVICE_ID_BT848) }, @@ -286,8 +272,7 @@ static struct pci_driver bt8xxgpio_pci_driver = { .id_table = bt8xxgpio_pci_tbl, .probe = bt8xxgpio_probe, .remove = bt8xxgpio_remove, - .suspend = bt8xxgpio_suspend, - .resume = bt8xxgpio_resume, + .driver.pm = &bt8xxgpio_pm_ops, }; module_pci_driver(bt8xxgpio_pci_driver); -- 2.51.0 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH v5] gpio: bt8xx: use generic PCI PM 2025-10-13 15:52 ` [PATCH v5] gpio: bt8xx: " Vaibhav Gupta @ 2025-10-13 17:43 ` Bjorn Helgaas 2025-10-16 16:36 ` [PATCH v6] gpio: bt8xx: use generic power management Vaibhav Gupta 2025-10-16 16:37 ` [PATCH v5] gpio: bt8xx: use generic PCI PM Vaibhav Gupta 0 siblings, 2 replies; 23+ messages in thread From: Bjorn Helgaas @ 2025-10-13 17:43 UTC (permalink / raw) To: Vaibhav Gupta Cc: Michael Buesch, Bartosz Golaszewski, Linus Walleij, linux-gpio, linux-kernel On Mon, Oct 13, 2025 at 03:52:59PM +0000, Vaibhav Gupta wrote: > Switch to the new generic PCI power management framework and remove legacy > callbacks like .suspend() and .resume(). With the generic framework, the > standard PCI related work like: > - pci_save/restore_state() > - pci_enable/disable_device() > - pci_set_power_state() > is handled by the PCI core and this driver should implement only gpio-bt8xx > specific operations in its respective callback functions. Tiny nits: s/use generic PCI PM/use generic power management/ # subject; not PCI s/new generic PCI power/generic power management/ # no longer "new" :) > Signed-off-by: Vaibhav Gupta <vaibhavgupta40@gmail.com> > --- > drivers/gpio/gpio-bt8xx.c | 29 +++++++---------------------- > 1 file changed, 7 insertions(+), 22 deletions(-) > > diff --git a/drivers/gpio/gpio-bt8xx.c b/drivers/gpio/gpio-bt8xx.c > index 05401da03ca3..70b49c385b43 100644 > --- a/drivers/gpio/gpio-bt8xx.c > +++ b/drivers/gpio/gpio-bt8xx.c > @@ -52,10 +52,8 @@ struct bt8xxgpio { > struct pci_dev *pdev; > struct gpio_chip gpio; > > -#ifdef CONFIG_PM > u32 saved_outen; > u32 saved_data; > -#endif > }; > > #define bgwrite(dat, adr) writel((dat), bg->mmio+(adr)) > @@ -224,9 +222,9 @@ static void bt8xxgpio_remove(struct pci_dev *pdev) > pci_disable_device(pdev); > } > > -#ifdef CONFIG_PM > -static int bt8xxgpio_suspend(struct pci_dev *pdev, pm_message_t state) > +static int __maybe_unused bt8xxgpio_suspend(struct device *dev) I think __maybe_unused is unnecessary because of this path: DEFINE_SIMPLE_DEV_PM_OPS _DEFINE_DEV_PM_OPS SYSTEM_SLEEP_PM_OPS pm_sleep_ptr PTR_IF(IS_ENABLED(CONFIG_PM_SLEEP), (_ptr)) Detailed explanation here: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/util_macros.h?id=v6.17#n86 > { > + struct pci_dev *pdev = to_pci_dev(dev); > struct bt8xxgpio *bg = pci_get_drvdata(pdev); > > scoped_guard(spinlock_irqsave, &bg->lock) { Unrelated to this patch, but it's not clear to me why bg->lock is needed here (or maybe anywhere). $ git grep -l "static int.*_suspend" drivers/gpio | wc -l 26 $ git grep -W "static int.*_suspend" drivers/gpio | grep lock | grep -v unlock | wc -l It looks like only about 8 of the 26 gpio drivers that implement .suspend() protect it with a lock. I would expect a lock to be needed in all of them or none of them. > @@ -238,23 +236,13 @@ static int bt8xxgpio_suspend(struct pci_dev *pdev, pm_message_t state) > bgwrite(0x0, BT848_GPIO_OUT_EN); > } > > - pci_save_state(pdev); > - pci_disable_device(pdev); > - pci_set_power_state(pdev, pci_choose_state(pdev, state)); > - > return 0; > } > > -static int bt8xxgpio_resume(struct pci_dev *pdev) > +static int __maybe_unused bt8xxgpio_resume(struct device *dev) > { > + struct pci_dev *pdev = to_pci_dev(dev); > struct bt8xxgpio *bg = pci_get_drvdata(pdev); > - int err; > - > - pci_set_power_state(pdev, PCI_D0); > - err = pci_enable_device(pdev); > - if (err) > - return err; > - pci_restore_state(pdev); > > guard(spinlock_irqsave)(&bg->lock); > > @@ -267,10 +255,8 @@ static int bt8xxgpio_resume(struct pci_dev *pdev) > > return 0; > } > -#else > -#define bt8xxgpio_suspend NULL > -#define bt8xxgpio_resume NULL > -#endif /* CONFIG_PM */ > + > +static DEFINE_SIMPLE_DEV_PM_OPS(bt8xxgpio_pm_ops, bt8xxgpio_suspend, bt8xxgpio_resume); > > static const struct pci_device_id bt8xxgpio_pci_tbl[] = { > { PCI_DEVICE(PCI_VENDOR_ID_BROOKTREE, PCI_DEVICE_ID_BT848) }, > @@ -286,8 +272,7 @@ static struct pci_driver bt8xxgpio_pci_driver = { > .id_table = bt8xxgpio_pci_tbl, > .probe = bt8xxgpio_probe, > .remove = bt8xxgpio_remove, > - .suspend = bt8xxgpio_suspend, > - .resume = bt8xxgpio_resume, > + .driver.pm = &bt8xxgpio_pm_ops, > }; > > module_pci_driver(bt8xxgpio_pci_driver); > -- > 2.51.0 > ^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v6] gpio: bt8xx: use generic power management 2025-10-13 17:43 ` Bjorn Helgaas @ 2025-10-16 16:36 ` Vaibhav Gupta 2025-10-21 8:48 ` Bartosz Golaszewski 2025-10-23 7:59 ` Bartosz Golaszewski 2025-10-16 16:37 ` [PATCH v5] gpio: bt8xx: use generic PCI PM Vaibhav Gupta 1 sibling, 2 replies; 23+ messages in thread From: Vaibhav Gupta @ 2025-10-16 16:36 UTC (permalink / raw) To: Michael Buesch, Bjorn Helgaas, Bartosz Golaszewski Cc: Vaibhav Gupta, Linus Walleij, linux-gpio, linux-kernel Switch to the generic PCI power management framework and remove legacy callbacks like .suspend() and .resume(). With the generic framework, the standard PCI related work like: - pci_save/restore_state() - pci_enable/disable_device() - pci_set_power_state() is handled by the PCI core and this driver should implement only gpio-bt8xx specific operations in its respective callback functions. Signed-off-by: Vaibhav Gupta <vaibhavgupta40@gmail.com> --- drivers/gpio/gpio-bt8xx.c | 30 ++++++++---------------------- 1 file changed, 8 insertions(+), 22 deletions(-) diff --git a/drivers/gpio/gpio-bt8xx.c b/drivers/gpio/gpio-bt8xx.c index 05401da03ca3..324eeb77dbd5 100644 --- a/drivers/gpio/gpio-bt8xx.c +++ b/drivers/gpio/gpio-bt8xx.c @@ -52,10 +52,8 @@ struct bt8xxgpio { struct pci_dev *pdev; struct gpio_chip gpio; -#ifdef CONFIG_PM u32 saved_outen; u32 saved_data; -#endif }; #define bgwrite(dat, adr) writel((dat), bg->mmio+(adr)) @@ -224,9 +222,10 @@ static void bt8xxgpio_remove(struct pci_dev *pdev) pci_disable_device(pdev); } -#ifdef CONFIG_PM -static int bt8xxgpio_suspend(struct pci_dev *pdev, pm_message_t state) + +static int bt8xxgpio_suspend(struct device *dev) { + struct pci_dev *pdev = to_pci_dev(dev); struct bt8xxgpio *bg = pci_get_drvdata(pdev); scoped_guard(spinlock_irqsave, &bg->lock) { @@ -238,23 +237,13 @@ static int bt8xxgpio_suspend(struct pci_dev *pdev, pm_message_t state) bgwrite(0x0, BT848_GPIO_OUT_EN); } - pci_save_state(pdev); - pci_disable_device(pdev); - pci_set_power_state(pdev, pci_choose_state(pdev, state)); - return 0; } -static int bt8xxgpio_resume(struct pci_dev *pdev) +static int bt8xxgpio_resume(struct device *dev) { + struct pci_dev *pdev = to_pci_dev(dev); struct bt8xxgpio *bg = pci_get_drvdata(pdev); - int err; - - pci_set_power_state(pdev, PCI_D0); - err = pci_enable_device(pdev); - if (err) - return err; - pci_restore_state(pdev); guard(spinlock_irqsave)(&bg->lock); @@ -267,10 +256,8 @@ static int bt8xxgpio_resume(struct pci_dev *pdev) return 0; } -#else -#define bt8xxgpio_suspend NULL -#define bt8xxgpio_resume NULL -#endif /* CONFIG_PM */ + +static DEFINE_SIMPLE_DEV_PM_OPS(bt8xxgpio_pm_ops, bt8xxgpio_suspend, bt8xxgpio_resume); static const struct pci_device_id bt8xxgpio_pci_tbl[] = { { PCI_DEVICE(PCI_VENDOR_ID_BROOKTREE, PCI_DEVICE_ID_BT848) }, @@ -286,8 +273,7 @@ static struct pci_driver bt8xxgpio_pci_driver = { .id_table = bt8xxgpio_pci_tbl, .probe = bt8xxgpio_probe, .remove = bt8xxgpio_remove, - .suspend = bt8xxgpio_suspend, - .resume = bt8xxgpio_resume, + .driver.pm = &bt8xxgpio_pm_ops, }; module_pci_driver(bt8xxgpio_pci_driver); -- 2.51.0 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH v6] gpio: bt8xx: use generic power management 2025-10-16 16:36 ` [PATCH v6] gpio: bt8xx: use generic power management Vaibhav Gupta @ 2025-10-21 8:48 ` Bartosz Golaszewski 2025-10-22 19:29 ` Bjorn Helgaas 2025-10-23 7:59 ` Bartosz Golaszewski 1 sibling, 1 reply; 23+ messages in thread From: Bartosz Golaszewski @ 2025-10-21 8:48 UTC (permalink / raw) To: Vaibhav Gupta Cc: Michael Buesch, Bjorn Helgaas, Linus Walleij, linux-gpio, linux-kernel On Thu, Oct 16, 2025 at 6:36 PM Vaibhav Gupta <vaibhavgupta40@gmail.com> wrote: > > Switch to the generic PCI power management framework and remove legacy > callbacks like .suspend() and .resume(). With the generic framework, the > standard PCI related work like: > - pci_save/restore_state() > - pci_enable/disable_device() > - pci_set_power_state() > is handled by the PCI core and this driver should implement only gpio-bt8xx > specific operations in its respective callback functions. > > Signed-off-by: Vaibhav Gupta <vaibhavgupta40@gmail.com> > --- This says it's a v6 but I have no idea what changed since v1. Please provide a changelog for every version when submitting patches. Bjorn: does this look good to you? Bartosz ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v6] gpio: bt8xx: use generic power management 2025-10-21 8:48 ` Bartosz Golaszewski @ 2025-10-22 19:29 ` Bjorn Helgaas 2025-10-23 7:49 ` Vaibhav Gupta 0 siblings, 1 reply; 23+ messages in thread From: Bjorn Helgaas @ 2025-10-22 19:29 UTC (permalink / raw) To: Bartosz Golaszewski Cc: Vaibhav Gupta, Michael Buesch, Linus Walleij, linux-gpio, linux-kernel On Tue, Oct 21, 2025 at 10:48:48AM +0200, Bartosz Golaszewski wrote: > On Thu, Oct 16, 2025 at 6:36 PM Vaibhav Gupta <vaibhavgupta40@gmail.com> wrote: > > > > Switch to the generic PCI power management framework and remove legacy > > callbacks like .suspend() and .resume(). With the generic framework, the > > standard PCI related work like: > > - pci_save/restore_state() > > - pci_enable/disable_device() > > - pci_set_power_state() > > is handled by the PCI core and this driver should implement only gpio-bt8xx > > specific operations in its respective callback functions. > > > > Signed-off-by: Vaibhav Gupta <vaibhavgupta40@gmail.com> > > --- > > This says it's a v6 but I have no idea what changed since v1. Please > provide a changelog for every version when submitting patches. > > Bjorn: does this look good to you? Yes, it looks good to me. Reviewed-by: Bjorn Helgaas <bhelgaas@google.com> FWIW, here's the diff from v1 to v6. Mostly just iterating on compile warning nits: diff --git a/drivers/gpio/gpio-bt8xx.c b/drivers/gpio/gpio-bt8xx.c index e8d0c67bb618..324eeb77dbd5 100644 --- a/drivers/gpio/gpio-bt8xx.c +++ b/drivers/gpio/gpio-bt8xx.c @@ -52,10 +52,8 @@ struct bt8xxgpio { struct pci_dev *pdev; struct gpio_chip gpio; -#ifdef CONFIG_PM u32 saved_outen; u32 saved_data; -#endif }; #define bgwrite(dat, adr) writel((dat), bg->mmio+(adr)) @@ -224,7 +222,8 @@ static void bt8xxgpio_remove(struct pci_dev *pdev) pci_disable_device(pdev); } -static int __maybe_unused bt8xxgpio_suspend(struct device *dev) + +static int bt8xxgpio_suspend(struct device *dev) { struct pci_dev *pdev = to_pci_dev(dev); struct bt8xxgpio *bg = pci_get_drvdata(pdev); @@ -241,7 +240,7 @@ static int __maybe_unused bt8xxgpio_suspend(struct device *dev) return 0; } -static int __maybe_unused bt8xxgpio_resume(struct device *dev) +static int bt8xxgpio_resume(struct device *dev) { struct pci_dev *pdev = to_pci_dev(dev); struct bt8xxgpio *bg = pci_get_drvdata(pdev); @@ -258,7 +257,7 @@ static int __maybe_unused bt8xxgpio_resume(struct device *dev) return 0; } -static SIMPLE_DEV_PM_OPS(bt8xxgpio_pm_ops, bt8xxgpio_suspend, bt8xxgpio_resume); +static DEFINE_SIMPLE_DEV_PM_OPS(bt8xxgpio_pm_ops, bt8xxgpio_suspend, bt8xxgpio_resume); static const struct pci_device_id bt8xxgpio_pci_tbl[] = { { PCI_DEVICE(PCI_VENDOR_ID_BROOKTREE, PCI_DEVICE_ID_BT848) }, ^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH v6] gpio: bt8xx: use generic power management 2025-10-22 19:29 ` Bjorn Helgaas @ 2025-10-23 7:49 ` Vaibhav Gupta 2025-10-23 7:55 ` Bartosz Golaszewski 0 siblings, 1 reply; 23+ messages in thread From: Vaibhav Gupta @ 2025-10-23 7:49 UTC (permalink / raw) To: Bjorn Helgaas, Bartosz Golaszewski Cc: Michael Buesch, Linus Walleij, linux-gpio, linux-kernel > > > > This says it's a v6 but I have no idea what changed since v1. Please > > provide a changelog for every version when submitting patches. > > > > Bjorn: does this look good to you? > > Yes, it looks good to me. > > Reviewed-by: Bjorn Helgaas <bhelgaas@google.com> > > FWIW, here's the diff from v1 to v6. Mostly just iterating on > compile warning nits: > > diff --git a/drivers/gpio/gpio-bt8xx.c b/drivers/gpio/gpio-bt8xx.c > index e8d0c67bb618..324eeb77dbd5 100644 > --- a/drivers/gpio/gpio-bt8xx.c > +++ b/drivers/gpio/gpio-bt8xx.c > @@ -52,10 +52,8 @@ struct bt8xxgpio { > struct pci_dev *pdev; > struct gpio_chip gpio; > > -#ifdef CONFIG_PM > u32 saved_outen; > u32 saved_data; > -#endif > }; > > #define bgwrite(dat, adr) writel((dat), bg->mmio+(adr)) > @@ -224,7 +222,8 @@ static void bt8xxgpio_remove(struct pci_dev *pdev) > pci_disable_device(pdev); > } > > -static int __maybe_unused bt8xxgpio_suspend(struct device *dev) > + > +static int bt8xxgpio_suspend(struct device *dev) > { > struct pci_dev *pdev = to_pci_dev(dev); > struct bt8xxgpio *bg = pci_get_drvdata(pdev); > @@ -241,7 +240,7 @@ static int __maybe_unused bt8xxgpio_suspend(struct device *dev) > return 0; > } > > -static int __maybe_unused bt8xxgpio_resume(struct device *dev) > +static int bt8xxgpio_resume(struct device *dev) > { > struct pci_dev *pdev = to_pci_dev(dev); > struct bt8xxgpio *bg = pci_get_drvdata(pdev); > @@ -258,7 +257,7 @@ static int __maybe_unused bt8xxgpio_resume(struct device *dev) > return 0; > } > > -static SIMPLE_DEV_PM_OPS(bt8xxgpio_pm_ops, bt8xxgpio_suspend, bt8xxgpio_resume); > +static DEFINE_SIMPLE_DEV_PM_OPS(bt8xxgpio_pm_ops, bt8xxgpio_suspend, bt8xxgpio_resume); > > static const struct pci_device_id bt8xxgpio_pci_tbl[] = { > { PCI_DEVICE(PCI_VENDOR_ID_BROOKTREE, PCI_DEVICE_ID_BT848) }, Hello Bjorn! Thanks for the review and mentioning the diff between v1 and v6. Hey Randy! Please let me know if the diff mentioned by Bjorn is enough or should I send a new patch-mail describing the v1-v6 diff? Best regards, Vaibhav ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v6] gpio: bt8xx: use generic power management 2025-10-23 7:49 ` Vaibhav Gupta @ 2025-10-23 7:55 ` Bartosz Golaszewski 2025-10-23 7:58 ` Vaibhav Gupta 0 siblings, 1 reply; 23+ messages in thread From: Bartosz Golaszewski @ 2025-10-23 7:55 UTC (permalink / raw) To: Vaibhav Gupta Cc: Bjorn Helgaas, Michael Buesch, Linus Walleij, linux-gpio, linux-kernel On Thu, Oct 23, 2025 at 9:49 AM Vaibhav Gupta <vaibhavgupta40@gmail.com> wrote: > > Hello Bjorn! > Thanks for the review and mentioning the diff between v1 and v6. > > Hey Randy! > Please let me know if the diff mentioned by Bjorn is enough or should I send a > new patch-mail describing the v1-v6 diff? > Yes, it's enough, I could have looked it up myself but I shouldn't have to. Please, next time just list changes under each new iteration. Preferably just use b4, it helps with version management. Bart ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v6] gpio: bt8xx: use generic power management 2025-10-23 7:55 ` Bartosz Golaszewski @ 2025-10-23 7:58 ` Vaibhav Gupta 0 siblings, 0 replies; 23+ messages in thread From: Vaibhav Gupta @ 2025-10-23 7:58 UTC (permalink / raw) To: Bartosz Golaszewski Cc: Bjorn Helgaas, Michael Buesch, Linus Walleij, linux-gpio, linux-kernel On Thu, Oct 23, 2025 at 09:55:39AM +0200, Bartosz Golaszewski wrote: > On Thu, Oct 23, 2025 at 9:49 AM Vaibhav Gupta <vaibhavgupta40@gmail.com> wrote: > > > > Hello Bjorn! > > Thanks for the review and mentioning the diff between v1 and v6. > > > > Hey Randy! > > Please let me know if the diff mentioned by Bjorn is enough or should I send a > > new patch-mail describing the v1-v6 diff? > > > > Yes, it's enough, I could have looked it up myself but I shouldn't > have to. Please, next time just list changes under each new iteration. > Preferably just use b4, it helps with version management. > > Bart Noted. My patches will be more clear from next time. Thanks for the reviews and comments. - Vaibhav ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v6] gpio: bt8xx: use generic power management 2025-10-16 16:36 ` [PATCH v6] gpio: bt8xx: use generic power management Vaibhav Gupta 2025-10-21 8:48 ` Bartosz Golaszewski @ 2025-10-23 7:59 ` Bartosz Golaszewski 1 sibling, 0 replies; 23+ messages in thread From: Bartosz Golaszewski @ 2025-10-23 7:59 UTC (permalink / raw) To: Michael Buesch, Bjorn Helgaas, Bartosz Golaszewski, Vaibhav Gupta Cc: Bartosz Golaszewski, Linus Walleij, linux-gpio, linux-kernel From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> On Thu, 16 Oct 2025 16:36:13 +0000, Vaibhav Gupta wrote: > Switch to the generic PCI power management framework and remove legacy > callbacks like .suspend() and .resume(). With the generic framework, the > standard PCI related work like: > - pci_save/restore_state() > - pci_enable/disable_device() > - pci_set_power_state() > is handled by the PCI core and this driver should implement only gpio-bt8xx > specific operations in its respective callback functions. > > [...] Applied, thanks! [1/1] gpio: bt8xx: use generic power management https://git.kernel.org/brgl/linux/c/d5376026f9269601e239545e2ec4aea0cc62bf2a Best regards, -- Bartosz Golaszewski <bartosz.golaszewski@linaro.org> ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v5] gpio: bt8xx: use generic PCI PM 2025-10-13 17:43 ` Bjorn Helgaas 2025-10-16 16:36 ` [PATCH v6] gpio: bt8xx: use generic power management Vaibhav Gupta @ 2025-10-16 16:37 ` Vaibhav Gupta 1 sibling, 0 replies; 23+ messages in thread From: Vaibhav Gupta @ 2025-10-16 16:37 UTC (permalink / raw) To: Bjorn Helgaas Cc: Michael Buesch, Bartosz Golaszewski, Linus Walleij, linux-gpio, linux-kernel On Mon, Oct 13, 2025 at 12:43:19PM -0500, Bjorn Helgaas wrote: > On Mon, Oct 13, 2025 at 03:52:59PM +0000, Vaibhav Gupta wrote: > > Switch to the new generic PCI power management framework and remove legacy > > callbacks like .suspend() and .resume(). With the generic framework, the > > standard PCI related work like: > > - pci_save/restore_state() > > - pci_enable/disable_device() > > - pci_set_power_state() > > is handled by the PCI core and this driver should implement only gpio-bt8xx > > specific operations in its respective callback functions. > > Tiny nits: > > s/use generic PCI PM/use generic power management/ # subject; not PCI > s/new generic PCI power/generic power management/ # no longer "new" :) > > > Signed-off-by: Vaibhav Gupta <vaibhavgupta40@gmail.com> > > --- > > drivers/gpio/gpio-bt8xx.c | 29 +++++++---------------------- > > 1 file changed, 7 insertions(+), 22 deletions(-) > > > > diff --git a/drivers/gpio/gpio-bt8xx.c b/drivers/gpio/gpio-bt8xx.c > > index 05401da03ca3..70b49c385b43 100644 > > --- a/drivers/gpio/gpio-bt8xx.c > > +++ b/drivers/gpio/gpio-bt8xx.c > > @@ -52,10 +52,8 @@ struct bt8xxgpio { > > struct pci_dev *pdev; > > struct gpio_chip gpio; > > > > -#ifdef CONFIG_PM > > u32 saved_outen; > > u32 saved_data; > > -#endif > > }; > > > > #define bgwrite(dat, adr) writel((dat), bg->mmio+(adr)) > > @@ -224,9 +222,9 @@ static void bt8xxgpio_remove(struct pci_dev *pdev) > > pci_disable_device(pdev); > > } > > > > -#ifdef CONFIG_PM > > -static int bt8xxgpio_suspend(struct pci_dev *pdev, pm_message_t state) > > +static int __maybe_unused bt8xxgpio_suspend(struct device *dev) > > I think __maybe_unused is unnecessary because of this path: > > DEFINE_SIMPLE_DEV_PM_OPS > _DEFINE_DEV_PM_OPS > SYSTEM_SLEEP_PM_OPS > pm_sleep_ptr > PTR_IF(IS_ENABLED(CONFIG_PM_SLEEP), (_ptr)) > > Detailed explanation here: > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/util_macros.h?id=v6.17#n86 > Fixed them in v6. > > { > > + struct pci_dev *pdev = to_pci_dev(dev); > > struct bt8xxgpio *bg = pci_get_drvdata(pdev); > > > > scoped_guard(spinlock_irqsave, &bg->lock) { > > Unrelated to this patch, but it's not clear to me why bg->lock is > needed here (or maybe anywhere). > > $ git grep -l "static int.*_suspend" drivers/gpio | wc -l > 26 > $ git grep -W "static int.*_suspend" drivers/gpio | grep lock | grep -v unlock | wc -l > > It looks like only about 8 of the 26 gpio drivers that implement > .suspend() protect it with a lock. I would expect a lock to be needed > in all of them or none of them. > I can have a look at them. regards, Vaibhav > > @@ -238,23 +236,13 @@ static int bt8xxgpio_suspend(struct pci_dev *pdev, pm_message_t state) > > bgwrite(0x0, BT848_GPIO_OUT_EN); > > } > > > > - pci_save_state(pdev); > > - pci_disable_device(pdev); > > - pci_set_power_state(pdev, pci_choose_state(pdev, state)); > > - > > return 0; > > } > > > > -static int bt8xxgpio_resume(struct pci_dev *pdev) > > +static int __maybe_unused bt8xxgpio_resume(struct device *dev) > > { > > + struct pci_dev *pdev = to_pci_dev(dev); > > struct bt8xxgpio *bg = pci_get_drvdata(pdev); > > - int err; > > - > > - pci_set_power_state(pdev, PCI_D0); > > - err = pci_enable_device(pdev); > > - if (err) > > - return err; > > - pci_restore_state(pdev); > > > > guard(spinlock_irqsave)(&bg->lock); > > > > @@ -267,10 +255,8 @@ static int bt8xxgpio_resume(struct pci_dev *pdev) > > > > return 0; > > } > > -#else > > -#define bt8xxgpio_suspend NULL > > -#define bt8xxgpio_resume NULL > > -#endif /* CONFIG_PM */ > > + > > +static DEFINE_SIMPLE_DEV_PM_OPS(bt8xxgpio_pm_ops, bt8xxgpio_suspend, bt8xxgpio_resume); > > > > static const struct pci_device_id bt8xxgpio_pci_tbl[] = { > > { PCI_DEVICE(PCI_VENDOR_ID_BROOKTREE, PCI_DEVICE_ID_BT848) }, > > @@ -286,8 +272,7 @@ static struct pci_driver bt8xxgpio_pci_driver = { > > .id_table = bt8xxgpio_pci_tbl, > > .probe = bt8xxgpio_probe, > > .remove = bt8xxgpio_remove, > > - .suspend = bt8xxgpio_suspend, > > - .resume = bt8xxgpio_resume, > > + .driver.pm = &bt8xxgpio_pm_ops, > > }; > > > > module_pci_driver(bt8xxgpio_pci_driver); > > -- > > 2.51.0 > > ^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2025-10-23 7:59 UTC | newest] Thread overview: 23+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-10-10 10:53 [PATCH v1] driver: gpio-bt8xx: use generic PCI PM Vaibhav Gupta 2025-10-11 1:43 ` kernel test robot 2025-10-11 10:26 ` Michael Büsch 2025-10-11 11:32 ` Vaibhav Gupta 2025-10-11 12:31 ` Michael Büsch 2025-10-11 12:37 ` Vaibhav Gupta 2025-10-11 12:57 ` [PATCH v3] " Vaibhav Gupta 2025-10-13 11:04 ` [PATCH v4] driver: gpio: bt8xx: " Vaibhav Gupta 2025-10-11 12:32 ` [PATCH v2] driver: gpio-bt8xx: " Vaibhav Gupta 2025-10-13 9:41 ` [PATCH v1] " Bartosz Golaszewski 2025-10-13 11:03 ` Vaibhav Gupta 2025-10-13 15:36 ` Bartosz Golaszewski 2025-10-13 15:51 ` Vaibhav Gupta 2025-10-13 15:52 ` [PATCH v5] gpio: bt8xx: " Vaibhav Gupta 2025-10-13 17:43 ` Bjorn Helgaas 2025-10-16 16:36 ` [PATCH v6] gpio: bt8xx: use generic power management Vaibhav Gupta 2025-10-21 8:48 ` Bartosz Golaszewski 2025-10-22 19:29 ` Bjorn Helgaas 2025-10-23 7:49 ` Vaibhav Gupta 2025-10-23 7:55 ` Bartosz Golaszewski 2025-10-23 7:58 ` Vaibhav Gupta 2025-10-23 7:59 ` Bartosz Golaszewski 2025-10-16 16:37 ` [PATCH v5] gpio: bt8xx: use generic PCI PM Vaibhav Gupta
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).