mbox series

[v7,0/3] clk: clkdev add managed lookup registrations

Message ID cover.1544177090.git.matti.vaittinen@fi.rohmeurope.com (mailing list archive)
Headers show
Series clk: clkdev add managed lookup registrations | expand

Message

Vaittinen, Matti Dec. 7, 2018, 11:09 a.m. UTC 
Series adds managed clkdev lookup interfaces and cleans few drivers

Few clk drivers appear to be leaking clkdev lookup registrations at
driver remove. The patch series adds devm versions of lookup
registrations and cleans up few drivers. Driver clean-up patches have
not been tested as I lack the HW. All testing and comments if
driver/device removal is even possible for changed drivers is highly
appreciated. If removal is not possible I will gladly drop the patches
from series - although leaking lookups may serve as bad example for new
developers =)

Changed drivers are:
clk-max77686 and clk-st

Please note that the patch #2 requires this change to work correctly:
https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git/commit/?h=clk-next&id=05502bf9eb7a7297f5fa6f1d17b169b3d5b53570

Changelog v7:
- rewmoved already applied of_provider patches and
  now independent bd718x7 patch from the series.
- No functional changes.

Changelog v6
- Drop 'devm_of_clk_add_parent_hw_provider'. Change
  'devm_of_clk_add_hw_provider' to look the parent device node for
  provider information if device's own node does not contain
  #clock-cells - property.
- Add kerneldoc in own patch.
- Remove NULL checks from devres match function for clkdev releasing
- Clean styling issues from clkdev.c

Changelog v5
- Split v4 patch 1. Place clkdev stuff to patch 1 and clk provider to patch 2
- Remove devm_clk_release_clkdev from devres.txt
- Added kerneldoc for managed provider registrations.
- Cleaned styling issues.

Changelog v4
- Add support for ROHM bd718x7 PMIC clock gate. Included in this patch
  series because it depends on managed interfaces added in patch 1.

Changelog v3
Address issues spotted by Krzysztof Kozlowski
- Drop patch 3 for clk-s3c2410-dclk as this device can never be removed
- Fix indentiation for clk-max77686
- Rest  of the patches are unchanged.

Changelog v2
Issue spotted by 0-Day test suite
- Add a stub function 'devm_of_clk_add_parent_hw_provider' for no OF config.
- patches 2-8 are unchanged.

This patch series is based on clk-next

---

Matti Vaittinen (3):
  clkdev: add managed clkdev lookup registration
  clk: clk-max77686: Clean clkdev lookup leak and use devm
  clk: clk-st: avoid clkdev lookup leak at remove

 Documentation/driver-model/devres.txt |   1 +
 drivers/clk/clk-max77686.c            |  28 ++-------
 drivers/clk/clkdev.c                  | 111 +++++++++++++++++++++++++++-------
 drivers/clk/x86/clk-st.c              |   3 +-
 include/linux/clkdev.h                |   4 ++
 5 files changed, 101 insertions(+), 46 deletions(-)

Comments

Vaittinen, Matti Jan. 31, 2019, 1:24 p.m. UTC | #1 
Hello All,

On Fri, Dec 07, 2018 at 01:09:00PM +0200, Matti Vaittinen wrote:
> Series adds managed clkdev lookup interfaces and cleans few drivers
> 
> Few clk drivers appear to be leaking clkdev lookup registrations at
> driver remove. The patch series adds devm versions of lookup
> registrations and cleans up few drivers. Driver clean-up patches have
> not been tested as I lack the HW. All testing and comments if
> driver/device removal is even possible for changed drivers is highly
> appreciated. If removal is not possible I will gladly drop the patches
> from series - although leaking lookups may serve as bad example for new
> developers =)
> 
> Changed drivers are:
> clk-max77686 and clk-st
> 
> Please note that the patch #2 requires this change to work correctly:
> https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git/commit/?h=clk-next&id=05502bf9eb7a7297f5fa6f1d17b169b3d5b53570

I guess the dependency mentioned abowe is already in (most) of the
trees. (I can't say for sure as I don't know what is the correct tree
for clkdev - is it linux-arm.git as Russel is maintaining clkdev? If
yes, then the commit 05502bf9eb7a7297f5fa6f1d17b169b3d5b53570
"clk: of-provider: look at parent if registered device has no
provider info" seems to be sitting in maser branch).

So should I rebase this series to some other tree and resend? Or is this
something that is not wanted?

Br,
	Matti Vaittinen
Russell King (Oracle) Jan. 31, 2019, 3:21 p.m. UTC | #2 
On Thu, Jan 31, 2019 at 03:24:52PM +0200, Matti Vaittinen wrote:
> Hello All,
> 
> On Fri, Dec 07, 2018 at 01:09:00PM +0200, Matti Vaittinen wrote:
> > Series adds managed clkdev lookup interfaces and cleans few drivers
> > 
> > Few clk drivers appear to be leaking clkdev lookup registrations at
> > driver remove. The patch series adds devm versions of lookup
> > registrations and cleans up few drivers. Driver clean-up patches have
> > not been tested as I lack the HW. All testing and comments if
> > driver/device removal is even possible for changed drivers is highly
> > appreciated. If removal is not possible I will gladly drop the patches
> > from series - although leaking lookups may serve as bad example for new
> > developers =)
> > 
> > Changed drivers are:
> > clk-max77686 and clk-st
> > 
> > Please note that the patch #2 requires this change to work correctly:
> > https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git/commit/?h=clk-next&id=05502bf9eb7a7297f5fa6f1d17b169b3d5b53570
> 
> I guess the dependency mentioned abowe is already in (most) of the
> trees. (I can't say for sure as I don't know what is the correct tree
> for clkdev - is it linux-arm.git as Russel is maintaining clkdev?

Yes, I'm supposed to be maintaining clkdev, but I'm busy with other
stuff (such as reorganising my network, helping people with SFP issues,
I'm supposed to be replying to Arend over a brcmfmac issue that has been
on-going since Christmas which I haven't yet been able to doing the next
test...)

> If yes, then the commit 05502bf9eb7a7297f5fa6f1d17b169b3d5b53570
> "clk: of-provider: look at parent if registered device has no
> provider info" seems to be sitting in maser branch).

It's there because it's part of the mainline kernel and has been
since v5.0-rc1.  So, if you depend on that commit, basing off
v5.0-rc1 is probably sane, unless there's something else that
conflicts.
Stephen Boyd Jan. 31, 2019, 7:38 p.m. UTC | #3 
Quoting Russell King - ARM Linux admin (2019-01-31 07:21:47)
> On Thu, Jan 31, 2019 at 03:24:52PM +0200, Matti Vaittinen wrote:
> > Hello All,
> > 
> > On Fri, Dec 07, 2018 at 01:09:00PM +0200, Matti Vaittinen wrote:
> > > Series adds managed clkdev lookup interfaces and cleans few drivers
> > > 
> > > Few clk drivers appear to be leaking clkdev lookup registrations at
> > > driver remove. The patch series adds devm versions of lookup
> > > registrations and cleans up few drivers. Driver clean-up patches have
> > > not been tested as I lack the HW. All testing and comments if
> > > driver/device removal is even possible for changed drivers is highly
> > > appreciated. If removal is not possible I will gladly drop the patches
> > > from series - although leaking lookups may serve as bad example for new
> > > developers =)
> > > 
> > > Changed drivers are:
> > > clk-max77686 and clk-st
> > > 
> > > Please note that the patch #2 requires this change to work correctly:
> > > https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git/commit/?h=clk-next&id=05502bf9eb7a7297f5fa6f1d17b169b3d5b53570
> > 
> > I guess the dependency mentioned abowe is already in (most) of the
> > trees. (I can't say for sure as I don't know what is the correct tree
> > for clkdev - is it linux-arm.git as Russel is maintaining clkdev?
> 
> Yes, I'm supposed to be maintaining clkdev, but I'm busy with other
> stuff (such as reorganising my network, helping people with SFP issues,
> I'm supposed to be replying to Arend over a brcmfmac issue that has been
> on-going since Christmas which I haven't yet been able to doing the next
> test...)
> 
> > If yes, then the commit 05502bf9eb7a7297f5fa6f1d17b169b3d5b53570
> > "clk: of-provider: look at parent if registered device has no
> > provider info" seems to be sitting in maser branch).
> 
> It's there because it's part of the mainline kernel and has been
> since v5.0-rc1.  So, if you depend on that commit, basing off
> v5.0-rc1 is probably sane, unless there's something else that
> conflicts.
> 

I can pick up the patches in the clk tree. Clk drivers will be the users
so it's probably simplest to merge the clkdev patch in clk tree and then
consumers can be updated at the same time in a patch stack on top of
that. Otherwise, if I can get a stable branch/tag to pile the clk driver
patches on I can apply them that way and then the clkdev patch can go
via Russell.
Vaittinen, Matti Feb. 1, 2019, 8:40 a.m. UTC | #4 
On Thu, Jan 31, 2019 at 11:38:37AM -0800, Stephen Boyd wrote:
> Quoting Russell King - ARM Linux admin (2019-01-31 07:21:47)
> > On Thu, Jan 31, 2019 at 03:24:52PM +0200, Matti Vaittinen wrote:
> > > Hello All,
> > > 
> > > On Fri, Dec 07, 2018 at 01:09:00PM +0200, Matti Vaittinen wrote:
> > > > Series adds managed clkdev lookup interfaces and cleans few drivers
> > > > 
> > > > Few clk drivers appear to be leaking clkdev lookup registrations at
> > > > driver remove. The patch series adds devm versions of lookup
> > > > registrations and cleans up few drivers. Driver clean-up patches have
> > > > not been tested as I lack the HW. All testing and comments if
> > > > driver/device removal is even possible for changed drivers is highly
> > > > appreciated. If removal is not possible I will gladly drop the patches
> > > > from series - although leaking lookups may serve as bad example for new
> > > > developers =)
> > > > 
> > > > Changed drivers are:
> > > > clk-max77686 and clk-st
> > > > 
> > > > Please note that the patch #2 requires this change to work correctly:
> > > > https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git/commit/?h=clk-next&id=05502bf9eb7a7297f5fa6f1d17b169b3d5b53570
> > > 
> > > I guess the dependency mentioned abowe is already in (most) of the
> > > trees. (I can't say for sure as I don't know what is the correct tree
> > > for clkdev - is it linux-arm.git as Russel is maintaining clkdev?
> > 
> > Yes, I'm supposed to be maintaining clkdev, but I'm busy with other
> > stuff (such as reorganising my network, helping people with SFP issues,
> > I'm supposed to be replying to Arend over a brcmfmac issue that has been
> > on-going since Christmas which I haven't yet been able to doing the next
> > test...)
> > 
> > > If yes, then the commit 05502bf9eb7a7297f5fa6f1d17b169b3d5b53570
> > > "clk: of-provider: look at parent if registered device has no
> > > provider info" seems to be sitting in maser branch).
> > 
> > It's there because it's part of the mainline kernel and has been
> > since v5.0-rc1.  So, if you depend on that commit, basing off
> > v5.0-rc1 is probably sane, unless there's something else that
> > conflicts.
> > 
> 
> I can pick up the patches in the clk tree. Clk drivers will be the users
> so it's probably simplest to merge the clkdev patch in clk tree and then
> consumers can be updated at the same time in a patch stack on top of
> that.

This sounds good to me if it is Ok to Russel. Please just let me know if
you want me to rebase (to clk-next?) and resend the series.

> Otherwise, if I can get a stable branch/tag to pile the clk driver
> patches on I can apply them that way and then the clkdev patch can go
> via Russell.

I can for sure go with this approach as well if it has some advantages
to you. Again, please just let me know if I should resend series and if
I should use something else but clk-next as a base.

Br,
	Matti Vaittinen