[PATCH v3 00/27] Add DM support for omap PWM backlight

Dario Binacchi dariobin at libero.it
Fri Oct 16 22:31:50 CEST 2020 

Hi Felix,

> Il 16/10/2020 10:41 Felix Brack <fb at ltec.ch> ha scritto:
> 
>  
> Hello Dario,
> 
> On 15.10.20 20:56, Dario Binacchi wrote:
> > Hi Felix,
> > 
> >> Il 15/10/2020 10:53 Felix Brack <fb at ltec.ch> ha scritto:
> >>
> >>  
> >> Hello Dario,
> >>
> >> On 14.10.20 23:22, Dario Binacchi wrote:
> >>>
> >>>> Il 14/10/2020 10:22 Felix Brack <fb at ltec.ch> ha scritto:
> >>>>
> >>>>  
> >>>> On 11.10.20 14:13, 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.
> >>>>>
> >>>>> As recommended by Grygorii Strashko I have removed some patches to
> >>>>> upload them in new different series:
> >>>>> - [v2,01/30] clk: remove a redundant header
> >>>>> - [v2,03/30] arch: sandbox: fix typo in clk.h
> >>>>> - [v2,20/30] video: backlight: fix pwm's duty cycle calculation
> >>>>> - [v2,21/30] video: backlight: fix pwm data structure description
> >>>>> - [v2,22/30] dm: core: improve uclass_get_device_by_phandle_id() description
> >>>>> - [v2,23/30] gpio: fix gpio_request_by_name() description
> >>>>>
> >>>>> Grygorii suggested to remove also other patches that I have kept in the
> >>>>> series to allow anyone who wants to be able to test it. Moreover after
> >>>>> the addition of a patch that has largely changed the device tree. The
> >>>>> patches removed are the one and only ones that do not affect the series
> >>>>> in any way. I hope that the next version of this series will require
> >>>>> only minimal changes allowing me to split it according to Grygorii's
> >>>>> suggestions.
> >>>>> ---- 
> >>>>
> >>>
> >>>> I must be missing something as this patch series does not apply without
> >>>> errors to U-Boot master:
> >>>>
> >>>
> >>> Hi Felix,
> >>> I applied the patch of the series downloaded from patchwork on master and I compiled it without errors.
> >>>
> >> Did you do that against a recently cloned master?
> >>
> >>>> error: drivers/clk/clk-ti-am3-dpll-x2.c: No such file or directory
> >>>> error: drivers/clk/clk-ti-am3-dpll.c: No such file or directory
> >>>> error: drivers/clk/clk-ti-ctrl.c: No such file or directory
> >>>> error: drivers/clk/clk-ti-divider.c: No such file or directory
> >>>> error: drivers/clk/clk-ti-gate.c: No such file or directory
> >>>> error: drivers/clk/clk-ti-mux.c: No such file or directory
> >>>> error: drivers/clk/clk-ti.c: No such file or directory
> >>>> error: drivers/clk/clk-ti.h: No such file or directory
> >>>
> >>> After applying the patch you should find:
> >>> ls drivers/clk/ti
> >>> am3-prcm.c
> >>> clk-am3-dpll.c
> >>> clk-am3-dpll-x2.c
> >>> clk.c
> >>> clk-ctrl.c
> >>> clk-divider.c
> >>> clk-gate.c
> >>> clk.h
> >>> clk-mux.c
> >>> clk-sci.c
> >>> Kconfig
> >>> Makefile
> >>> omap4-cm.c
> >>>
> >>>> error: drivers/video/tilcdc-panel.c: No such file or directory
> >>>> error: drivers/video/tilcdc-panel.h: No such file or directory
> >>>> error: drivers/video/tilcdc.c: No such file or directory
> >>>> error: drivers/video/tilcdc.h: No such file or directory
> >>>>
> >>>
> >>> and 
> >>> ls drivers/video/ti
> >>> am335x-fb.c
> >>> am335x-fb.h
> >>> Kconfig
> >>> Makefile
> >>> tilcdc.c
> >>> tilcdc.h
> >>> tilcdc-panel.c
> >>> tilcdc-panel.h
> >>>
> >> As the patch series does not apply I don't see these files.
> >>
> >>>> Does it probably depend on some other patches?
> >>> No, the series patch does not depend on other patches
> >>>>
> >>>> regards, Felix
> >>>
> >>> Best regards
> >>> Dario
> >>>
> >>
> >> Two hours ago I did:
> >>
> >> 1. git clone https://gitlab.denx.de/u-boot/u-boot.git
> >> 2. downloaded Add-DM-support-for-omap-PWM-backlight.patch from
> >>    https://patchwork.ozlabs.org/project/uboot/list/?series=207124
> >> 3. git apply Add-DM-support-for-omap-PWM-backlight.patch --check -v
> >>
> >> result:
> >>
> >> Checking patch drivers/clk/clk-divider.c...
> >> Checking patch include/linux/clk-provider.h...
> >> Checking patch include/dt-bindings/bus/ti-sysc.h...
> >> ...
> >> Checking patch drivers/clk/ti/Kconfig...
> >> Checking patch drivers/clk/ti/Makefile...
> >> Checking patch drivers/clk/clk-ti-am3-dpll-x2.c =>
> >> drivers/clk/ti/clk-am3-dpll-x2.c...
> >> error: drivers/clk/clk-ti-am3-dpll-x2.c: No such file or directory
> >> Checking patch drivers/clk/clk-ti-am3-dpll.c =>
> >> drivers/clk/ti/clk-am3-dpll.c...
> >> error: drivers/clk/clk-ti-am3-dpll.c: No such file or directory
> >> Checking patch drivers/clk/clk-ti-ctrl.c => drivers/clk/ti/clk-ctrl.c...
> >> error: drivers/clk/clk-ti-ctrl.c: No such file or directory
> >> Checking patch drivers/clk/clk-ti-divider.c =>
> >> drivers/clk/ti/clk-divider.c...
> >> error: drivers/clk/clk-ti-divider.c: No such file or directory
> >> Checking patch drivers/clk/clk-ti-gate.c => drivers/clk/ti/clk-gate.c...
> >> error: drivers/clk/clk-ti-gate.c: No such file or directory
> >> Checking patch drivers/clk/clk-ti-mux.c => drivers/clk/ti/clk-mux.c...
> >> error: drivers/clk/clk-ti-mux.c: No such file or directory
> >> Checking patch drivers/clk/clk-ti-sci.c => drivers/clk/ti/clk-sci.c...
> >> Checking patch drivers/clk/clk-ti.c => drivers/clk/ti/clk.c...
> >> error: drivers/clk/clk-ti.c: No such file or directory
> >> Checking patch drivers/clk/clk-ti.h => drivers/clk/ti/clk.h...
> >> error: drivers/clk/clk-ti.h: No such file or directory
> >> Checking patch drivers/clk/ti/Makefile...
> >> Checking patch drivers/clk/ti/omap4-cm.c...
> >> Checking patch drivers/clk/ti/Makefile...
> >> Checking patch drivers/clk/ti/am3-prcm.c...
> >> Checking patch arch/sandbox/dts/test.dts...
> >> Checking patch common/fdt_support.c...
> >> Checking patch drivers/core/Kconfig...
> >> Checking patch drivers/core/fdtaddr.c...
> >> Checking patch drivers/core/of_addr.c...
> >> Checking patch drivers/core/ofnode.c...
> >> Checking patch drivers/core/root.c...
> >> Checking patch include/asm-generic/global_data.h...
> >> error: while searching for:
> >>         struct global_data *new_gd;     /* relocated global data */
> >>
> >> #ifdef CONFIG_DM
> >>         struct udevice  *dm_root;       /* Root instance for Driver Model */
> >>         struct udevice  *dm_root_f;     /* Pre-relocation root instance */
> >>         struct list_head uclass_root;   /* Head of core tree */
> >>
> >> error: patch failed: include/asm-generic/global_data.h:64
> >> error: include/asm-generic/global_data.h: patch does not apply
> >> Checking patch test/dm/test-fdt.c...
> >> Checking patch drivers/timer/omap-timer.c...
> >> Checking patch drivers/misc/Kconfig...
> >> ...
> >> Checking patch drivers/video/ti/Kconfig...
> >> Checking patch drivers/video/ti/Makefile...
> >> Checking patch drivers/video/am335x-fb.c => drivers/video/ti/am335x-fb.c...
> >> Checking patch drivers/video/am335x-fb.h => drivers/video/ti/am335x-fb.h...
> >> Checking patch drivers/video/tilcdc-panel.c =>
> >> drivers/video/ti/tilcdc-panel.c...
> >> error: drivers/video/tilcdc-panel.c: No such file or directory
> >> Checking patch drivers/video/tilcdc-panel.h =>
> >> drivers/video/ti/tilcdc-panel.h...
> >> error: drivers/video/tilcdc-panel.h: No such file or directory
> >> Checking patch drivers/video/tilcdc.c => drivers/video/ti/tilcdc.c...
> >> error: drivers/video/tilcdc.c: No such file or directory
> >> Checking patch drivers/video/tilcdc.h => drivers/video/ti/tilcdc.h...
> >> error: drivers/video/tilcdc.h: No such file or directory
> >> Checking patch board/ti/am335x/board.c...
> >> Checking patch board/ti/am43xx/board.c...
> >>
> >> Applying the patches 1 to 15 manually works but then patch 16 fails with:
> >>
> >> Checking patch arch/sandbox/dts/test.dts...
> >> Checking patch common/fdt_support.c...
> >> Checking patch drivers/core/Kconfig...
> >> Checking patch drivers/core/fdtaddr.c...
> >> Checking patch drivers/core/of_addr.c...
> >> Checking patch drivers/core/ofnode.c...
> >> Checking patch drivers/core/root.c...
> >> Checking patch include/asm-generic/global_data.h...
> >> error: while searching for:
> >>         struct global_data *new_gd;     /* relocated global data */
> >>
> >> #ifdef CONFIG_DM
> >>         struct udevice  *dm_root;       /* Root instance for Driver Model */
> >>         struct udevice  *dm_root_f;     /* Pre-relocation root instance */
> >>         struct list_head uclass_root;   /* Head of core tree */
> >>
> >> error: patch failed: include/asm-generic/global_data.h:64
> >> error: include/asm-generic/global_data.h: patch does not apply
> >> Checking patch test/dm/test-fdt.c...
> >>
> >> The include/asm-generic/global_data.h in the master does not have a line
> >> reading 'struct global_data *new_gd;     /* relocated global data */'.
> >> Line 180 of the master include/asm-generic/global_data.h file however
> >> reads as follows: 'struct global_data *new_gd;'
> >>
> >> regards Felix
> > 
> > Try with:
> > 1. cd /tmp
> > 2. git clone https://gitlab.denx.de/u-boot/u-boot.git
> > 3. cd /tmp/u-boot
> > 4. git checkout -b omap-pwm 9885313b9a
> No, I won't. 

Commit 9885313b9a was the HEAD of the master branch no more than a week ago when I uploaded the series. The series is made up of 27 patches, so it is not surprising that there is some conflict a week later if you want to apply the series to the HEAD of the master branch. As you can see, the commit 9885313b9a is dated 12 October 2020.

> It will be a pleasure to me testing this patch series on
> the board I'm maintaining, 

I appreciate it and I thank you

> but only after it applies cleanly to the HEAD
> of the U-Boot master branch. 

However if this is a problem, I will rebase the patch series on master and upload it to patchwork as version 4.

> Meanwhile I suggest you add a comment about
> the required starting point in the repository to all future versions of
> this patch series.

I'll do it for the next versions.

> 
> > 5. git am Add-DM-support-for-omap-PWM-backlight.patch
> > 
> > Regards,
> > Dario
> > 
> 
> regards, Felix

Regards,
Dario

More information about the U-Boot mailing list