mbox series

[v3,0/3] drivers: ide: use generic power management

Message ID 20200713173613.2095-1-vaibhavgupta40@gmail.com
Headers show
Series drivers: ide: use generic power management | expand

Message

Vaibhav Gupta July 13, 2020, 5:36 p.m. UTC
Linux Kernel Mentee: Remove Legacy Power Management.

The purpose of this patch series is to remove legacy power management callbacks
from ide drivers.

The suspend() and resume() callbacks operations are still invoking
pci_save/restore_state(), pci_set_power_state(), pci_enable/disable_state(),
etc. and handling the power management themselves, which is not recommended.

The conversion requires the removal of the those function calls and change the
callback definition accordingly and make use of dev_pm_ops structure.

All patches are compile-tested only.

v3:
    - Modpost error for undefined reference by Kbuild in v1.
    - Another approach to disable PM in drivers/ide/triflex.c suggested by
      Bjorn Helgaas in v2.

Test tools:
    - Compiler: gcc (GCC) 10.1.0
    - allmodconfig build: make -j$(nproc) W=1 all

Vaibhav Gupta (3):
  ide: use generic power management
  ide: sc1200: use generic power management
  ide: delkin_cb: use generic power management

 drivers/ide/aec62xx.c         |  3 +--
 drivers/ide/alim15x3.c        |  3 +--
 drivers/ide/amd74xx.c         |  3 +--
 drivers/ide/atiixp.c          |  3 +--
 drivers/ide/cmd64x.c          |  3 +--
 drivers/ide/cs5520.c          |  3 +--
 drivers/ide/cs5530.c          |  3 +--
 drivers/ide/cs5535.c          |  3 +--
 drivers/ide/cs5536.c          |  3 +--
 drivers/ide/cy82c693.c        |  3 +--
 drivers/ide/delkin_cb.c       | 32 +++++---------------------
 drivers/ide/hpt366.c          |  3 +--
 drivers/ide/ide-pci-generic.c |  3 +--
 drivers/ide/it8172.c          |  3 +--
 drivers/ide/it8213.c          |  3 +--
 drivers/ide/it821x.c          |  3 +--
 drivers/ide/jmicron.c         |  3 +--
 drivers/ide/ns87415.c         |  3 +--
 drivers/ide/opti621.c         |  3 +--
 drivers/ide/pdc202xx_new.c    |  3 +--
 drivers/ide/pdc202xx_old.c    |  3 +--
 drivers/ide/piix.c            |  3 +--
 drivers/ide/sc1200.c          | 43 ++++++++++++-----------------------
 drivers/ide/serverworks.c     |  3 +--
 drivers/ide/setup-pci.c       | 29 +++++------------------
 drivers/ide/siimage.c         |  3 +--
 drivers/ide/sis5513.c         |  3 +--
 drivers/ide/sl82c105.c        |  3 +--
 drivers/ide/slc90e66.c        |  3 +--
 drivers/ide/triflex.c         | 24 +++++++------------
 drivers/ide/via82cxxx.c       |  3 +--
 include/linux/ide.h           |  8 +------
 32 files changed, 62 insertions(+), 155 deletions(-)

Comments

Bartlomiej Zolnierkiewicz July 14, 2020, 7:32 a.m. UTC | #1
Hi,

On 7/13/20 7:36 PM, Vaibhav Gupta wrote:
> Linux Kernel Mentee: Remove Legacy Power Management.
> 
> The purpose of this patch series is to remove legacy power management callbacks
> from ide drivers.
> 
> The suspend() and resume() callbacks operations are still invoking
> pci_save/restore_state(), pci_set_power_state(), pci_enable/disable_state(),
> etc. and handling the power management themselves, which is not recommended.
> 
> The conversion requires the removal of the those function calls and change the
> callback definition accordingly and make use of dev_pm_ops structure.

IDE subsystem (drivers/ide/) is deprecated and has been superseded by libata
subsystem (drivers/ata/).

libata drivers have the same issue so please concentrate on fixing them
first. Later (if desirable) changes can be back-ported to drivers/ide/.

> All patches are compile-tested only.

This patchset needs (at least) some basic testing. It should be easier with
libata subsystem as it also support SATA controllers and devices.

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

> v3:
>     - Modpost error for undefined reference by Kbuild in v1.
>     - Another approach to disable PM in drivers/ide/triflex.c suggested by
>       Bjorn Helgaas in v2.
> 
> Test tools:
>     - Compiler: gcc (GCC) 10.1.0
>     - allmodconfig build: make -j$(nproc) W=1 all
> 
> Vaibhav Gupta (3):
>   ide: use generic power management
>   ide: sc1200: use generic power management
>   ide: delkin_cb: use generic power management
> 
>  drivers/ide/aec62xx.c         |  3 +--
>  drivers/ide/alim15x3.c        |  3 +--
>  drivers/ide/amd74xx.c         |  3 +--
>  drivers/ide/atiixp.c          |  3 +--
>  drivers/ide/cmd64x.c          |  3 +--
>  drivers/ide/cs5520.c          |  3 +--
>  drivers/ide/cs5530.c          |  3 +--
>  drivers/ide/cs5535.c          |  3 +--
>  drivers/ide/cs5536.c          |  3 +--
>  drivers/ide/cy82c693.c        |  3 +--
>  drivers/ide/delkin_cb.c       | 32 +++++---------------------
>  drivers/ide/hpt366.c          |  3 +--
>  drivers/ide/ide-pci-generic.c |  3 +--
>  drivers/ide/it8172.c          |  3 +--
>  drivers/ide/it8213.c          |  3 +--
>  drivers/ide/it821x.c          |  3 +--
>  drivers/ide/jmicron.c         |  3 +--
>  drivers/ide/ns87415.c         |  3 +--
>  drivers/ide/opti621.c         |  3 +--
>  drivers/ide/pdc202xx_new.c    |  3 +--
>  drivers/ide/pdc202xx_old.c    |  3 +--
>  drivers/ide/piix.c            |  3 +--
>  drivers/ide/sc1200.c          | 43 ++++++++++++-----------------------
>  drivers/ide/serverworks.c     |  3 +--
>  drivers/ide/setup-pci.c       | 29 +++++------------------
>  drivers/ide/siimage.c         |  3 +--
>  drivers/ide/sis5513.c         |  3 +--
>  drivers/ide/sl82c105.c        |  3 +--
>  drivers/ide/slc90e66.c        |  3 +--
>  drivers/ide/triflex.c         | 24 +++++++------------
>  drivers/ide/via82cxxx.c       |  3 +--
>  include/linux/ide.h           |  8 +------
>  32 files changed, 62 insertions(+), 155 deletions(-)
>
Vaibhav Gupta July 14, 2020, 7:52 a.m. UTC | #2
On Tue, Jul 14, 2020 at 09:32:56AM +0200, Bartlomiej Zolnierkiewicz wrote:
> 
> Hi,
> 
> On 7/13/20 7:36 PM, Vaibhav Gupta wrote:
> > Linux Kernel Mentee: Remove Legacy Power Management.
> > 
> > The purpose of this patch series is to remove legacy power management callbacks
> > from ide drivers.
> > 
> > The suspend() and resume() callbacks operations are still invoking
> > pci_save/restore_state(), pci_set_power_state(), pci_enable/disable_state(),
> > etc. and handling the power management themselves, which is not recommended.
> > 
> > The conversion requires the removal of the those function calls and change the
> > callback definition accordingly and make use of dev_pm_ops structure.
> 
> IDE subsystem (drivers/ide/) is deprecated and has been superseded by libata
> subsystem (drivers/ata/).
> 
> libata drivers have the same issue so please concentrate on fixing them
> first. Later (if desirable) changes can be back-ported to drivers/ide/.
>
Hello, (drivers/ide) and (drivers/ata) are the two major families, I am working
on, for generic PM upgradation. I was bit unaware about priority, and also in
the last, both ide and ata drivers have to be upgraded.
> > All patches are compile-tested only.
> 
> This patchset needs (at least) some basic testing. It should be easier with
> libata subsystem as it also support SATA controllers and devices.
To upgrade PM in (drivers/ide) I have made .suspend() and .resume() static. Then
bind them in "struct dev_pm_ops" variable (ide_pci_pm_ops) and expose it using
EXPORT_SYMBOL_GPL(). This has affected 30 drivers. I was hoping if ide changes
can be tested/verified, specially [PATCH 1/3]. As then, I will be sure about
similar change in ata, as it also requires similar alteration.

Thanks
Vaibhav Gupta

> 
> Best regards,
> --
> Bartlomiej Zolnierkiewicz
> Samsung R&D Institute Poland
> Samsung Electronics
> 
> > v3:
> >     - Modpost error for undefined reference by Kbuild in v1.
> >     - Another approach to disable PM in drivers/ide/triflex.c suggested by
> >       Bjorn Helgaas in v2.
> > 
> > Test tools:
> >     - Compiler: gcc (GCC) 10.1.0
> >     - allmodconfig build: make -j$(nproc) W=1 all
> > 
> > Vaibhav Gupta (3):
> >   ide: use generic power management
> >   ide: sc1200: use generic power management
> >   ide: delkin_cb: use generic power management
> > 
> >  drivers/ide/aec62xx.c         |  3 +--
> >  drivers/ide/alim15x3.c        |  3 +--
> >  drivers/ide/amd74xx.c         |  3 +--
> >  drivers/ide/atiixp.c          |  3 +--
> >  drivers/ide/cmd64x.c          |  3 +--
> >  drivers/ide/cs5520.c          |  3 +--
> >  drivers/ide/cs5530.c          |  3 +--
> >  drivers/ide/cs5535.c          |  3 +--
> >  drivers/ide/cs5536.c          |  3 +--
> >  drivers/ide/cy82c693.c        |  3 +--
> >  drivers/ide/delkin_cb.c       | 32 +++++---------------------
> >  drivers/ide/hpt366.c          |  3 +--
> >  drivers/ide/ide-pci-generic.c |  3 +--
> >  drivers/ide/it8172.c          |  3 +--
> >  drivers/ide/it8213.c          |  3 +--
> >  drivers/ide/it821x.c          |  3 +--
> >  drivers/ide/jmicron.c         |  3 +--
> >  drivers/ide/ns87415.c         |  3 +--
> >  drivers/ide/opti621.c         |  3 +--
> >  drivers/ide/pdc202xx_new.c    |  3 +--
> >  drivers/ide/pdc202xx_old.c    |  3 +--
> >  drivers/ide/piix.c            |  3 +--
> >  drivers/ide/sc1200.c          | 43 ++++++++++++-----------------------
> >  drivers/ide/serverworks.c     |  3 +--
> >  drivers/ide/setup-pci.c       | 29 +++++------------------
> >  drivers/ide/siimage.c         |  3 +--
> >  drivers/ide/sis5513.c         |  3 +--
> >  drivers/ide/sl82c105.c        |  3 +--
> >  drivers/ide/slc90e66.c        |  3 +--
> >  drivers/ide/triflex.c         | 24 +++++++------------
> >  drivers/ide/via82cxxx.c       |  3 +--
> >  include/linux/ide.h           |  8 +------
> >  32 files changed, 62 insertions(+), 155 deletions(-)
> > 
>
Bartlomiej Zolnierkiewicz July 14, 2020, 8:19 a.m. UTC | #3
On 7/14/20 9:52 AM, Vaibhav Gupta wrote:
> On Tue, Jul 14, 2020 at 09:32:56AM +0200, Bartlomiej Zolnierkiewicz wrote:
>>
>> Hi,
>>
>> On 7/13/20 7:36 PM, Vaibhav Gupta wrote:
>>> Linux Kernel Mentee: Remove Legacy Power Management.
>>>
>>> The purpose of this patch series is to remove legacy power management callbacks
>>> from ide drivers.
>>>
>>> The suspend() and resume() callbacks operations are still invoking
>>> pci_save/restore_state(), pci_set_power_state(), pci_enable/disable_state(),
>>> etc. and handling the power management themselves, which is not recommended.
>>>
>>> The conversion requires the removal of the those function calls and change the
>>> callback definition accordingly and make use of dev_pm_ops structure.
>>
>> IDE subsystem (drivers/ide/) is deprecated and has been superseded by libata
>> subsystem (drivers/ata/).
>>
>> libata drivers have the same issue so please concentrate on fixing them
>> first. Later (if desirable) changes can be back-ported to drivers/ide/.
>>
> Hello, (drivers/ide) and (drivers/ata) are the two major families, I am working
> on, for generic PM upgradation. I was bit unaware about priority, and also in
> the last, both ide and ata drivers have to be upgraded.

Well, drivers/ide/ is scheduled for removal in 2021
(it even prints the warning during initialization of
every host driver)..

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

>>> All patches are compile-tested only.
>>
>> This patchset needs (at least) some basic testing. It should be easier with
>> libata subsystem as it also support SATA controllers and devices.
> To upgrade PM in (drivers/ide) I have made .suspend() and .resume() static. Then
> bind them in "struct dev_pm_ops" variable (ide_pci_pm_ops) and expose it using
> EXPORT_SYMBOL_GPL(). This has affected 30 drivers. I was hoping if ide changes
> can be tested/verified, specially [PATCH 1/3]. As then, I will be sure about
> similar change in ata, as it also requires similar alteration.
> 
> Thanks
> Vaibhav Gupta
> 
>>
>> Best regards,
>> --
>> Bartlomiej Zolnierkiewicz
>> Samsung R&D Institute Poland
>> Samsung Electronics
>>
>>> v3:
>>>     - Modpost error for undefined reference by Kbuild in v1.
>>>     - Another approach to disable PM in drivers/ide/triflex.c suggested by
>>>       Bjorn Helgaas in v2.
>>>
>>> Test tools:
>>>     - Compiler: gcc (GCC) 10.1.0
>>>     - allmodconfig build: make -j$(nproc) W=1 all
>>>
>>> Vaibhav Gupta (3):
>>>   ide: use generic power management
>>>   ide: sc1200: use generic power management
>>>   ide: delkin_cb: use generic power management
>>>
>>>  drivers/ide/aec62xx.c         |  3 +--
>>>  drivers/ide/alim15x3.c        |  3 +--
>>>  drivers/ide/amd74xx.c         |  3 +--
>>>  drivers/ide/atiixp.c          |  3 +--
>>>  drivers/ide/cmd64x.c          |  3 +--
>>>  drivers/ide/cs5520.c          |  3 +--
>>>  drivers/ide/cs5530.c          |  3 +--
>>>  drivers/ide/cs5535.c          |  3 +--
>>>  drivers/ide/cs5536.c          |  3 +--
>>>  drivers/ide/cy82c693.c        |  3 +--
>>>  drivers/ide/delkin_cb.c       | 32 +++++---------------------
>>>  drivers/ide/hpt366.c          |  3 +--
>>>  drivers/ide/ide-pci-generic.c |  3 +--
>>>  drivers/ide/it8172.c          |  3 +--
>>>  drivers/ide/it8213.c          |  3 +--
>>>  drivers/ide/it821x.c          |  3 +--
>>>  drivers/ide/jmicron.c         |  3 +--
>>>  drivers/ide/ns87415.c         |  3 +--
>>>  drivers/ide/opti621.c         |  3 +--
>>>  drivers/ide/pdc202xx_new.c    |  3 +--
>>>  drivers/ide/pdc202xx_old.c    |  3 +--
>>>  drivers/ide/piix.c            |  3 +--
>>>  drivers/ide/sc1200.c          | 43 ++++++++++++-----------------------
>>>  drivers/ide/serverworks.c     |  3 +--
>>>  drivers/ide/setup-pci.c       | 29 +++++------------------
>>>  drivers/ide/siimage.c         |  3 +--
>>>  drivers/ide/sis5513.c         |  3 +--
>>>  drivers/ide/sl82c105.c        |  3 +--
>>>  drivers/ide/slc90e66.c        |  3 +--
>>>  drivers/ide/triflex.c         | 24 +++++++------------
>>>  drivers/ide/via82cxxx.c       |  3 +--
>>>  include/linux/ide.h           |  8 +------
>>>  32 files changed, 62 insertions(+), 155 deletions(-)
>>>
>>
> 
>
Vaibhav Gupta July 14, 2020, 8:24 a.m. UTC | #4
On Tue, Jul 14, 2020 at 10:19:06AM +0200, Bartlomiej Zolnierkiewicz wrote:
> 
> On 7/14/20 9:52 AM, Vaibhav Gupta wrote:
> > On Tue, Jul 14, 2020 at 09:32:56AM +0200, Bartlomiej Zolnierkiewicz wrote:
> >>
> >>
> >> IDE subsystem (drivers/ide/) is deprecated and has been superseded by libata
> >> subsystem (drivers/ata/).
> >>
> >> libata drivers have the same issue so please concentrate on fixing them
> >> first. Later (if desirable) changes can be back-ported to drivers/ide/.
> >>
> > Hello, (drivers/ide) and (drivers/ata) are the two major families, I am working
> > on, for generic PM upgradation. I was bit unaware about priority, and also in
> > the last, both ide and ata drivers have to be upgraded.
> 
> Well, drivers/ide/ is scheduled for removal in 2021
> (it even prints the warning during initialization of
> every host driver)..
Okay then, I will start with ata family.

Thanks,
Vaibhav Gupta
> 
> Best regards,
> --
> Bartlomiej Zolnierkiewicz
> Samsung R&D Institute Poland
> Samsung Electronics
> 
> >>> All patches are compile-tested only.
> >>
> >> This patchset needs (at least) some basic testing. It should be easier with
> >> libata subsystem as it also support SATA controllers and devices.
> > To upgrade PM in (drivers/ide) I have made .suspend() and .resume() static. Then
> > bind them in "struct dev_pm_ops" variable (ide_pci_pm_ops) and expose it using
> > EXPORT_SYMBOL_GPL(). This has affected 30 drivers. I was hoping if ide changes
> > can be tested/verified, specially [PATCH 1/3]. As then, I will be sure about
> > similar change in ata, as it also requires similar alteration.
> > 
> > Thanks
> > Vaibhav Gupta
> > 
> >>
> >> Best regards,
> >> --
> >> Bartlomiej Zolnierkiewicz
> >> Samsung R&D Institute Poland
> >> Samsung Electronics
> >>
David Miller Aug. 4, 2020, 1:49 a.m. UTC | #5
From: Vaibhav Gupta <vaibhavgupta40@gmail.com>
Date: Mon, 13 Jul 2020 23:06:10 +0530

> Linux Kernel Mentee: Remove Legacy Power Management.
> 
> The purpose of this patch series is to remove legacy power management callbacks
> from ide drivers.
> 
> The suspend() and resume() callbacks operations are still invoking
> pci_save/restore_state(), pci_set_power_state(), pci_enable/disable_state(),
> etc. and handling the power management themselves, which is not recommended.
> 
> The conversion requires the removal of the those function calls and change the
> callback definition accordingly and make use of dev_pm_ops structure.

Series applied.