[U-Boot] [PATCH v4 00/13] clk: Port Linux common clock framework [CCF] to U-boot (tag: 5.0-rc3)

Mon May 20 15:35:35 UTC 2019

Hi Lukasz,

On Sun, 19 May 2019 at 08:35, Lukasz Majewski <lukma at denx.de> wrote:
>
> Hi Simon,
>
> > Hi Lukasz,
> >
> > On Sat, 18 May 2019 at 14:49, Lukasz Majewski <lukma at denx.de> wrote:
> > >
> > > Hi Simon,
> > >
> > > > kHi Lukasz,
> > > >
> > > > On Thu, 16 May 2019 at 16:11, Lukasz Majewski <lukma at denx.de>
> > > > wrote:
> > > > >
> > > > > This patch series brings the files from Linux kernel to provide
> > > > > clocks support as it is used on the Linux kernel with common
> > > > > clock framework [CCF] setup.
> > > > >
> > > > > This series also fixes several problems with current clocks and
> > > > > provides sandbox tests for functions addded to clk-uclass.c
> > > > > file.
> > > > >
> > > > > Repository:
> > > > > https://github.com/lmajewski/u-boot-dfu/commits/CCF-v4
> > > > >
> > > > >
> > > > > Changes in v4:
> > > > > - New patch
> > > > > - None
> > > > > - None
> > > > > - None
> > > > > - New patch
> > > > > - None
> > > > > - Port some more Linux code to facilitate imx8 code porting
> > > > > (most notably flags)
> > > > > - Explicitly use container_of() based macro to provide struct
> > > > > clk in various places (e.g. gate2, mux, etc)
> > > > >   Following patches has been squashed:
> > > > >   http://patchwork.ozlabs.org/patch/1093141/
> > > > >   http://patchwork.ozlabs.org/patch/1093142/
> > > > >   http://patchwork.ozlabs.org/patch/1093146/
> > > > > - New patch
> > > > >
> > > > > Changes in v3:
> > > > > - New patch
> > > > > - The rate information is now cached into struct clk field
> > > > > - The clk_get_parent() is used to get pointer to the parent
> > > > > struct clk
> > > > > - Replace -ENODEV with -ENOENT
> > > > > - Use **clkp instead of **c
> > > > > - New patch
> > > > > - New patch
> > > > >
> > > > > Lukasz Majewski (13):
> > > > >   clk: doc: Add documentation entry for Common Clock Framework
> > > > > [CCF] (i.MX)
> > > > >   dm: Fix documentation entry as there is no UCLASS_CLOCK uclass
> > > > >   clk: Remove clock ID check in .get_rate() of clk_fixed_*
> > > > >   clk: Extend struct clk to provide information regarding clock
> > > > > rate clk: Extend struct clk to provide clock type agnostic flags
> > > > >   clk: Provide struct clk for fixed rate clock
> > > > > (clk_fixed_rate.c) dm: clk: Define clk_get_parent() for clk
> > > > > operations dm: clk: Define clk_get_parent_rate() for clk
> > > > > operations dm: clk: Define clk_get_by_id() for clk operations
> > > > >   clk: test: Provide unit test for clk_get_by_id() method
> > > > >   clk: test: Provide unit test for clk_get_parent_rate() method
> > > > >   clk: Port Linux common clock framework [CCF] for imx6q to
> > > > > U-boot (tag: 5.0-rc3)
> > > > >   dm: clk: Extend clk_get_parent_rate() to support
> > > > > CLK_GET_RATE_NOCACHE flag
> > > > >
> > > > >  arch/sandbox/include/asm/clk.h |  16 ++++
> > > > >  doc/imx/clk/ccf.txt            |  83 +++++++++++++++++++
> > > > >  drivers/clk/Kconfig            |  14 ++++
> > > > >  drivers/clk/Makefile           |   2 +
> > > > >  drivers/clk/clk-divider.c      | 148
> > > > > ++++++++++++++++++++++++++++++++++
> > > > > drivers/clk/clk-fixed-factor.c |  87 ++++++++++++++++++++
> > > > > drivers/clk/clk-mux.c          | 164
> > > > > +++++++++++++++++++++++++++++++++++++
> > > > > drivers/clk/clk-uclass.c       |  60 ++++++++++++++
> > > > > drivers/clk/clk.c              |  56 +++++++++++++
> > > > > drivers/clk/clk_fixed_factor.c |   3 -
> > > > > drivers/clk/clk_fixed_rate.c   |   8 +-
> > > > > drivers/clk/clk_sandbox_test.c |  49 +++++++++++
> > > > > drivers/clk/imx/Kconfig        |   9 +++
> > > > > drivers/clk/imx/Makefile       |   2 +
> > > > > drivers/clk/imx/clk-gate2.c    | 113 ++++++++++++++++++++++++++
> > > > > drivers/clk/imx/clk-imx6q.c    | 179
> > > > > +++++++++++++++++++++++++++++++++++++++++
> > > > > drivers/clk/imx/clk-pfd.c      |  91 +++++++++++++++++++++
> > > > > drivers/clk/imx/clk-pllv3.c    |  83 +++++++++++++++++++
> > > > > drivers/clk/imx/clk.h          |  75 +++++++++++++++++
> > > > > include/clk.h                  |  37 ++++++++-
> > > > > include/linux/clk-provider.h   | 115 ++++++++++++++++++++++++++
> > > > > test/dm/clk.c                  |   4 +- 22 files changed, 1390
> > > > > insertions(+), 8 deletions(-) create mode 100644
> > > > > doc/imx/clk/ccf.txt create mode 100644
> > > > > drivers/clk/clk-divider.c create mode 100644
> > > > > drivers/clk/clk-fixed-factor.c create mode 100644
> > > > > drivers/clk/clk-mux.c create mode 100644 drivers/clk/clk.c
> > > > > create mode 100644 drivers/clk/imx/clk-gate2.c create mode
> > > > > 100644 drivers/clk/imx/clk-imx6q.c create mode 100644
> > > > > drivers/clk/imx/clk-pfd.c create mode 100644
> > > > > drivers/clk/imx/clk-pllv3.c create mode 100644
> > > > > drivers/clk/imx/clk.h create mode 100644
> > > > > include/linux/clk-provider.h
> > > >
> > > > I don't see any new tests here other a trivial change in
> > > > test/dm/clk.c
> > > >
> > > > When adding new functionality to the generic clock code, tests
> > > > should be added to cover this code. The sandbox clock driver
> > > > should be enhanced as needed.
> > >
> > > Please correct me if I'm wrong (or provide not enough code for those
> > > use cases), but I've added some tests here:
> > >
> > > http://patchwork.ozlabs.org/patch/1100767/
> > > http://patchwork.ozlabs.org/patch/1100769/
> > >
> > > Are those not exhaustive enough? Have I overlooked something?
> >
> > Sorry...I even looked at those patches but missed that they were
> > tests. I think it would be better if the test code were in test/dm
> > instead of drivers/clk/clk_sandbox_test.c.
>
> I've added those tests in the same place as original clk_get_rate().
> However, there is no issue from my side to add them to test/dm if this
> is a better place.
>
> >
> > I made a comment about the driver_data thing though.
> >
> > I wonder if we should try to put tests (code in test/dm/... at least)
> > in with the change that introduces new functionality? Or would that be
> > too painful?
>
> As I said above - if the test/dm/ directory is a better place (than
> drivers/clk/clk_sandbox_test.c) to place CCF (clk) related tests - then
> I'm fine with it.

Well that file is strange. It seems to be a driver and not a test. But
why can't it use the normal sandbox clock drivers?

Regards,
Simon