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

[Grygorii Strashko]

On 17/09/2020 22:23, Dario Binacchi wrote:
> 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?
> 

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.

-- 
Best regards,
grygorii