| Message ID | 20200906120911.750-1-dariobin@libero.it |
|---|---|
| Headers | show |
| Series | Add DM support for omap PWM backlight | expand |
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'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. > > 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 >
Hi Grygorii, > Il 17/09/2020 08:57 Grygorii Strashko <grygorii.strashko@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
On 17/09/2020 22:23, Dario Binacchi wrote: > Hi Grygorii, > >> Il 17/09/2020 08:57 Grygorii Strashko <grygorii.strashko@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? > It depends, if you porting code to u-boot it's most probably that kernel bindings evolved already (>1 commit). In such case link on file may be preferable, i think. if it's new driver which just has been accepted to the Kernel with bindings - it could be sha1. Note. Common practice for u-boot is to accept new drivers only after their binding accepted in Linux Kernel.
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. 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 arch/arm/dts/am335x-brppt1-mmc.dts | 3 +- arch/arm/dts/am335x-brppt1-nand.dts | 3 +- arch/arm/dts/am335x-brppt1-spi.dts | 3 +- arch/arm/dts/am335x-brsmarc1.dts | 2 +- arch/arm/dts/am335x-brxre1.dts | 3 +- arch/arm/dts/am335x-evm-u-boot.dtsi | 7 +- arch/arm/dts/am335x-evmsk-u-boot.dtsi | 6 +- arch/arm/dts/am335x-guardian-u-boot.dtsi | 8 +- arch/arm/dts/am335x-pdu001-u-boot.dtsi | 8 +- arch/arm/dts/am335x-pxm50-u-boot.dtsi | 6 +- arch/arm/dts/am335x-rut-u-boot.dtsi | 6 +- arch/arm/dts/am33xx.dtsi | 12 + arch/arm/dts/da850-evm-u-boot.dtsi | 8 +- arch/arm/include/asm/arch-am33xx/clock.h | 1 + arch/arm/mach-omap2/am33xx/clock.c | 10 +- arch/arm/mach-omap2/am33xx/clock_am33xx.c | 2 +- arch/sandbox/dts/test.dts | 67 +++ arch/sandbox/include/asm/clk.h | 35 +- board/ti/am335x/board.c | 2 +- board/ti/am43xx/board.c | 2 +- common/fdt_support.c | 6 +- doc/device-tree-bindings/arm/omap,ctrl.txt | 82 +++ doc/device-tree-bindings/arm/omap,prcm.txt | 63 +++ .../clock/clock-bindings.txt | 186 +++++++ .../clock/gpio-gate-clock.txt | 21 + .../clock/ti,autoidle.txt | 39 ++ doc/device-tree-bindings/clock/ti,clkctrl.txt | 61 +++ .../clock/ti,clockdomain.txt | 24 + doc/device-tree-bindings/clock/ti,divider.txt | 117 +++++ doc/device-tree-bindings/clock/ti,dpll.txt | 85 ++++ doc/device-tree-bindings/clock/ti,gate.txt | 106 ++++ doc/device-tree-bindings/clock/ti,mux.txt | 79 +++ .../pinctrl/pinctrl-single.txt | 255 ++++++++++ doc/device-tree-bindings/pwm/ti,ehrpwm.txt | 49 ++ doc/device-tree-bindings/pwm/ti,pwmss.txt | 58 +++ drivers/clk/Kconfig | 9 +- drivers/clk/Makefile | 2 +- drivers/clk/clk-divider.c | 24 +- drivers/clk/clk-uclass.c | 15 + drivers/clk/clk_sandbox.c | 17 + drivers/clk/clk_sandbox_test.c | 10 + drivers/clk/ti/Kconfig | 42 ++ drivers/clk/ti/Makefile | 18 + drivers/clk/ti/am3-prcm.c | 65 +++ drivers/clk/ti/clk-am3-dpll-x2.c | 78 +++ drivers/clk/ti/clk-am3-dpll.c | 267 ++++++++++ drivers/clk/ti/clk-ctrl.c | 102 ++++ drivers/clk/ti/clk-divider.c | 380 ++++++++++++++ drivers/clk/ti/clk-gate.c | 92 ++++ drivers/clk/ti/clk-mux.c | 252 ++++++++++ drivers/clk/{clk-ti-sci.c => ti/clk-sci.c} | 0 drivers/clk/ti/clk.c | 34 ++ drivers/clk/ti/clk.h | 13 + drivers/clk/ti/omap4-cm.c | 22 + drivers/core/Kconfig | 12 + drivers/core/fdtaddr.c | 2 +- drivers/core/of_addr.c | 14 +- drivers/core/ofnode.c | 7 +- drivers/core/read.c | 6 + drivers/core/root.c | 3 + drivers/misc/Kconfig | 7 + drivers/misc/Makefile | 1 + drivers/misc/ti-am3-scm.c | 81 +++ drivers/pwm/Kconfig | 13 + drivers/pwm/Makefile | 2 + drivers/pwm/pwm-ti-ehrpwm.c | 465 ++++++++++++++++++ drivers/pwm/pwm-ti-pwmss.c | 95 ++++ drivers/timer/omap-timer.c | 6 +- drivers/video/Kconfig | 5 +- drivers/video/Makefile | 2 +- drivers/video/pwm_backlight.c | 4 +- drivers/video/ti/Kconfig | 8 + drivers/video/ti/Makefile | 10 + drivers/video/{ => ti}/am335x-fb.c | 342 +------------ drivers/video/{ => ti}/am335x-fb.h | 4 - drivers/video/ti/tilcdc-panel.c | 171 +++++++ drivers/video/ti/tilcdc-panel.h | 14 + drivers/video/ti/tilcdc.c | 438 +++++++++++++++++ drivers/video/ti/tilcdc.h | 38 ++ include/asm-generic/global_data.h | 6 + include/asm-generic/gpio.h | 2 +- include/clk-uclass.h | 8 + include/clk.h | 29 ++ include/dm/read.h | 24 + include/dm/uclass.h | 3 +- include/linux/clk-provider.h | 58 ++- test/dm/clk.c | 22 + test/dm/panel.c | 12 +- test/dm/test-fdt.c | 148 +++++- 89 files changed, 4488 insertions(+), 441 deletions(-) create mode 100644 doc/device-tree-bindings/arm/omap,ctrl.txt create mode 100644 doc/device-tree-bindings/arm/omap,prcm.txt create mode 100644 doc/device-tree-bindings/clock/clock-bindings.txt create mode 100644 doc/device-tree-bindings/clock/gpio-gate-clock.txt create mode 100644 doc/device-tree-bindings/clock/ti,autoidle.txt create mode 100644 doc/device-tree-bindings/clock/ti,clkctrl.txt create mode 100644 doc/device-tree-bindings/clock/ti,clockdomain.txt create mode 100644 doc/device-tree-bindings/clock/ti,divider.txt create mode 100644 doc/device-tree-bindings/clock/ti,dpll.txt create mode 100644 doc/device-tree-bindings/clock/ti,gate.txt create mode 100644 doc/device-tree-bindings/clock/ti,mux.txt create mode 100644 doc/device-tree-bindings/pinctrl/pinctrl-single.txt create mode 100644 doc/device-tree-bindings/pwm/ti,ehrpwm.txt create mode 100644 doc/device-tree-bindings/pwm/ti,pwmss.txt create mode 100644 drivers/clk/ti/Kconfig create mode 100644 drivers/clk/ti/Makefile create mode 100644 drivers/clk/ti/am3-prcm.c create mode 100644 drivers/clk/ti/clk-am3-dpll-x2.c create mode 100644 drivers/clk/ti/clk-am3-dpll.c create mode 100644 drivers/clk/ti/clk-ctrl.c create mode 100644 drivers/clk/ti/clk-divider.c create mode 100644 drivers/clk/ti/clk-gate.c create mode 100644 drivers/clk/ti/clk-mux.c rename drivers/clk/{clk-ti-sci.c => ti/clk-sci.c} (100%) create mode 100644 drivers/clk/ti/clk.c create mode 100644 drivers/clk/ti/clk.h create mode 100644 drivers/clk/ti/omap4-cm.c create mode 100644 drivers/misc/ti-am3-scm.c create mode 100644 drivers/pwm/pwm-ti-ehrpwm.c create mode 100644 drivers/pwm/pwm-ti-pwmss.c create mode 100644 drivers/video/ti/Kconfig create mode 100644 drivers/video/ti/Makefile rename drivers/video/{ => ti}/am335x-fb.c (54%) rename drivers/video/{ => ti}/am335x-fb.h (97%) create mode 100644 drivers/video/ti/tilcdc-panel.c create mode 100644 drivers/video/ti/tilcdc-panel.h create mode 100644 drivers/video/ti/tilcdc.c create mode 100644 drivers/video/ti/tilcdc.h