[PATCH v2 00/30] Add DM support for omap PWM backlight

[Dario Binacchi]

Hi Grygorii,

> Il 17/09/2020 08:57 Grygorii Strashko <grygorii.strashko at ti.com> ha scritto:
> 
>  
> Hi Dario,
> 
> On 06/09/2020 15:08, Dario Binacchi wrote:
> > 
> > The series was born from the need to manage the PWM backlight of the
> > display connected to my beaglebone board. To hit the target, I had to
> > develop drivers for PWM management which in turn relied on drivers for
> > managing timers and clocks, all developed according to the driver model.
> > My intention was to use the SoC-specific API only at strictly necessary
> > points in the code. My previous patches for migrating the AM335x display
> > driver to the driver model had required the implementation of additional
> > functions outside the concerns of the driver, (settings for dividing the
> > pixel clock rate, configuring the display DPLL rate, ....) not being
> > able to use the API of the related clock drivers. This series shouldn't
> > have repeated the same kind of mistake. Furthermore, I also wanted to fix
> > that kind of forced choice. Almost everything should have been accessible
> > via the driver model API. In the series there are also some patches that
> > could be submitted separately, but which I have however inserted to avoid
> > applying future patches to incorporate them.
> > With this last consideration, I hope I have convincingly justified the
> > large number of patches in the series.
> > 
> > The patch enabling address translation into a CPU physical address from
> > device-tree even in case of crossing levels with #size-cells = <0>, is
> > crucial for the series. The previous implementation was unable to
> > perform the address translation required by the am33xx device tree.
> > I tried to apply in a conservative way as few changes as possible and
> > to verify the execution of all the tests already developed, as well as
> > the new ones I added for the new feature.
> 
> Thank you for you patches.
> 
> In my opinion it's better if you split this series as it is
> - too big
> - modifies different subsystems
> - contains as fixes as new features

I agree with you. 
I've been thinking about it for some time too.
Hope in the weekend. Anyway, next patches upload
will split this series.
> 
> I'd recommend to separate
> - fixes first, like
>    clk: remove a redundant header
>    arch: sandbox: fix typo in clk.h
>    fdt: translate address if #size-cells = <0>
>    omap: timer: fix the rate setting
>    dm: core: add a function to decode display timings
>    ..
> - clk patches
> - pwm/backlight patches
> - video: omap: panel patches
> 
> And I'd recommend not to port device tree bindings in u-boot as it's just duplication of
> kernel binding which u-boot shell follow.
> Just providing links to Kernel binding in commit messages should be enough.

I have already added device-tree bindings in patches that have been accepted. No one has ever
pointed out what you recommend to me. Also, the doc/device-tree-bindings directory seems very 
crowded. I have read that device-tree bindings are often evolving and I think that not copying
them in uboot does not favor their consultation. Also I wonder if it is enough to report in the
commit message the kernel file path or to refer to a particular file version by specifying the 
commit sha1. Can you help me figure out what to do?

Best regards,
Dario
> 
> > 
> > Changes in v2:
> > - Add the clk_ prefix to the divider functions.
> > - Add kernel-doc comments to the exported functions.
> > - Merged to patch [09/31] clk: ti: refactor mux and divider clock
> >    drivers.
> > - Remove the 'ti_am3_prcm_clocks' driver. Handle 'prcm_clocks' node in
> >    the 'ti_am3_prcm' driver.
> > - Update the commit message.
> > - Fix a missing line in the commit message.
> > - Add dm_flags to global_data structure and GD_DM_FLG_SIZE_CELLS_0 macro
> >    to test without recompiling.
> > - Update the OF_CHECK_COUNTS macro in order to have just one
> >    #define by bringing the GD_DM_FLG_SIZE_CELLS_0 into the expression.
> > - Lower-case the 0xC019 hex number.
> > - Remove the 'ti_am3_scm_clocks' driver. Handle 'scm_clocks' node in
> >    the 'ti_am3_scm' driver.
> > - Update the commit message.
> > 
> > Dario Binacchi (30):
> >    clk: remove a redundant header
> >    clk: export generic routines
> >    arch: sandbox: fix typo in clk.h
> >    clk: add clk_round_rate()
> >    clk: ti: add mux clock driver
> >    arm: ti: am33xx: add DPLL_EN_FAST_RELOCK_BYPASS macro
> >    clk: ti: am33xx: add DPLL clock drivers
> >    clk: ti: add divider clock driver
> >    clk: ti: add gate clock driver
> >    ti: am33xx: fix do_enable_clocks() to accept NULL parameters
> >    clk: ti: add support for clkctrl clocks
> >    clk: ti: move drivers to 'ti' directory
> >    clk: ti: omap4: add clock manager driver
> >    clk: ti: am335x: add clock manager driver
> >    fdt: translate address if #size-cells = <0>
> >    omap: timer: fix the rate setting
> >    misc: am33xx: add control module driver
> >    pwm: ti: am33xx: add enhanced pwm driver
> >    pwm: ti: am33xx: add subsystem driver
> >    video: backlight: fix pwm's duty cycle calculation
> >    video: backlight: fix pwm data structure description
> >    dm: core: improve uclass_get_device_by_phandle_id() description
> >    gpio: fix gpio_request_by_name() description
> >    dm: core: add a function to decode display timings
> >    video: omap: add panel driver
> >    video: omap: enable LCD clock domain through DM API
> >    video: omap: set LCD clock rate through DM API
> >    video: omap: split the legacy code from the DM code
> >    video: omap: move drivers to 'ti' directory
> >    board: ti: am335x-ice: get CDCE913 clock device
> > 
> 
> 
> -- 
> Best regards,
> grygorii