[PATCH v2 3/3] ARM: dts: rockchip: add rk3288 ISP and DPHY

[Robin Murphy]

On 2020-07-02 17:27, Helen Koike wrote:
[...]
> I suggest this:
> 
>    clocks:
>      maxItems: 5
>      minItems: 3
>      description:
>        rk3288 clocks
>          ISP clock
>          ISP AXI clock
>          ISP AHB clock
>          ISP Pixel clock
>          ISP JPEG source clock
>        rk3399 isp0 clocks
>          ISP clock
>          ISP AXI wrapper clock
>          ISP AHB wrapper clock
>        rk3399 isp1 clocks
>          ISP clock
>          ISP AXI wrapper clock
>          ISP AHB wrapper clock
>          ISP Pixel wrapper clock
> 
>    clock-names:
>      oneOf:
>        # rk3288 clocks
>        - items:
>          - const: clk_isp
>          - const: aclk_isp
>          - const: hclk_isp
>          - const: pclk_isp_in
>          - const: sclk_isp_jpe
>        # rk3399 isp0 clocks
>        - items:
>          - const: clk_isp
>          - const: aclk_isp_wrap
>          - const: hclk_isp_wrap
>        # rk3399 isp1 clocks
>        - items:
>          - const: clk_isp
>          - const: aclk_isp_wrap
>          - const: hclk_isp_wrap
>          - const: pclk_isp_wrap

FWIW this looks a little more involved than it might need to be. Ideally 
we're describing things from the point of view of what inputs the device 
itself wants, so the details of exactly *how* a particular SoC's clock 
tree delivers them shouldn't matter to the binding, only to the actual 
clock specifier values ultimately given in the DT.

 From the ISP's PoV, it seems like we've got the fairly standard core 
clock, ACLK and HCLK trio, plus a pixel clock for RK3288 and RK3399 
ISP1, plus a JPEG source clock for RK3288. I'd be inclined to model that 
as simply something like:

     clock-names:
       minItems: 3
       maxItems: 5
       items:
       - const: isp
       - const: aclk
       - const: hclk
       - const: pclk
       - const: sclk_jpe

Plus then not only do we have a nice clean binding, but we avoid all the 
unnecessary faff of having to deal with the "same" clocks by different 
names in drivers, and sidestep the conundrum of what to do when the next 
SoC comes along providing the basic ISP clocks from yet again 
slightly-differently-named branches of its clock tree.

Robin.