From: Kieran Bingham <[email protected]>
The r8a77995 D3 platform has 2 LVDS channels connected to the DU.
Signed-off-by: Kieran Bingham <[email protected]>
---
arch/arm64/boot/dts/renesas/r8a77995.dtsi | 52 +++++++++++++++++++++++++++++++
1 file changed, 52 insertions(+)
diff --git a/arch/arm64/boot/dts/renesas/r8a77995.dtsi b/arch/arm64/boot/dts/renesas/r8a77995.dtsi
index a57d5fecf79c..7851e37cfff1 100644
--- a/arch/arm64/boot/dts/renesas/r8a77995.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a77995.dtsi
@@ -773,18 +773,70 @@
port@1 {
reg = <1>;
du_out_lvds0: endpoint {
+ remote-endpoint = <&du_out_lvds0>;
};
};
port@2 {
reg = <2>;
du_out_lvds1: endpoint {
+ remote-endpoint = <&du_out_lvds1>;
};
};
};
};
};
+ lvds0: lvds@feb90000 {
+ compatible = "renesas,lvds-r8a77995";
+ reg = <0 0xfeb90000 0 0x20>;
+ clocks = <&cpg CPG_MOD 727>;
+ status = "disabled";
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+ lvds0_in: endpoint {
+ remote-endpoint = <&du_out_lvds0>;
+ };
+ };
+
+ port@1 {
+ reg = <1>;
+ lvds0_out: endpoint {
+ };
+ };
+ };
+ };
+
+ lvds1: lvds@feb90100 {
+ compatible = "renesas,lvds-r8a77995";
+ reg = <0 0xfeb90100 0 0x20>;
+ clocks = <&cpg CPG_MOD 727>;
+ status = "disabled";
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+ lvds1_in: endpoint {
+ remote-endpoint = <&du_out_lvds1>;
+ };
+ };
+
+ port@1 {
+ reg = <1>;
+ lvds1_out: endpoint {
+ };
+ };
+ };
+ };
+
timer {
compatible = "arm,armv8-timer";
interrupts-extended = <&gic GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(1) | IRQ_TYPE_LEVEL_LOW)>,
--
2.7.4
Hi Kieran,
On Thu, Feb 15, 2018 at 9:38 AM, Kieran Bingham <[email protected]> wrote:
> From: Kieran Bingham <[email protected]>
>
> The r8a77995 D3 platform has 2 LVDS channels connected to the DU.
>
> Signed-off-by: Kieran Bingham <[email protected]>
I love your patch!
Yet something to improve...
> --- a/arch/arm64/boot/dts/renesas/r8a77995.dtsi
> +++ b/arch/arm64/boot/dts/renesas/r8a77995.dtsi
> @@ -773,18 +773,70 @@
> port@1 {
> reg = <1>;
> du_out_lvds0: endpoint {
> + remote-endpoint = <&du_out_lvds0>;
> };
> };
>
> port@2 {
> reg = <2>;
> du_out_lvds1: endpoint {
> + remote-endpoint = <&du_out_lvds1>;
> };
> };
> };
> };
> };
>
> + lvds0: lvds@feb90000 {
> + compatible = "renesas,lvds-r8a77995";
> + reg = <0 0xfeb90000 0 0x20>;
> + clocks = <&cpg CPG_MOD 727>;
> + status = "disabled";
Missing resets, power-domains
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Hi Geert,
On 15/02/18 09:07, Geert Uytterhoeven wrote:
> Hi Kieran,
>
> On Thu, Feb 15, 2018 at 9:38 AM, Kieran Bingham <[email protected]> wrote:
>> From: Kieran Bingham <[email protected]>
>>
>> The r8a77995 D3 platform has 2 LVDS channels connected to the DU.
>>
>> Signed-off-by: Kieran Bingham <[email protected]>
>
> I love your patch!
> Yet something to improve...
>
>> --- a/arch/arm64/boot/dts/renesas/r8a77995.dtsi
>> +++ b/arch/arm64/boot/dts/renesas/r8a77995.dtsi
>> @@ -773,18 +773,70 @@
>> port@1 {
>> reg = <1>;
>> du_out_lvds0: endpoint {
>> + remote-endpoint = <&du_out_lvds0>;
>> };
>> };
>>
>> port@2 {
>> reg = <2>;
>> du_out_lvds1: endpoint {
>> + remote-endpoint = <&du_out_lvds1>;
>> };
>> };
>> };
>> };
>> };
>>
>> + lvds0: lvds@feb90000 {
>> + compatible = "renesas,lvds-r8a77995";
>> + reg = <0 0xfeb90000 0 0x20>;
>> + clocks = <&cpg CPG_MOD 727>;
>> + status = "disabled";
>
> Missing resets, power-domains
Both lvds0, and lvds1 are actually the same 'module' (just different mappings
within the same IP block).
That means they will share clock, reset etc.
Will specifying the resets cause one to affect the other unduly?
like if one becomes un-used, will it power down? or is it refcounted, and will
gracefully handle both device nodes from the same device?
I'm already fairly sure clocks is fine for this - but it's just the reset's that
worry me. If the reset module tries to reset lvds0 for some reason - would lvds1
get reset as a side effect ...
Or perhaps that won't ever happen with an :
power-domains = <&sysc R8A77995_PD_ALWAYS_ON>;
resets = <&cpg 727>;
Regards
--
Kieran
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
> -- Linus Torvalds
>
Hi Kieran,
On Thu, Feb 15, 2018 at 1:41 PM, Kieran Bingham
<[email protected]> wrote:
> On 15/02/18 09:07, Geert Uytterhoeven wrote:
>> On Thu, Feb 15, 2018 at 9:38 AM, Kieran Bingham <[email protected]> wrote:
>>> From: Kieran Bingham <[email protected]>
>>>
>>> The r8a77995 D3 platform has 2 LVDS channels connected to the DU.
>>>
>>> Signed-off-by: Kieran Bingham <[email protected]>
>>
>> I love your patch!
>> Yet something to improve...
>>
>>> --- a/arch/arm64/boot/dts/renesas/r8a77995.dtsi
>>> +++ b/arch/arm64/boot/dts/renesas/r8a77995.dtsi
>>> @@ -773,18 +773,70 @@
>>> port@1 {
>>> reg = <1>;
>>> du_out_lvds0: endpoint {
>>> + remote-endpoint = <&du_out_lvds0>;
>>> };
>>> };
>>>
>>> port@2 {
>>> reg = <2>;
>>> du_out_lvds1: endpoint {
>>> + remote-endpoint = <&du_out_lvds1>;
>>> };
>>> };
>>> };
>>> };
>>> };
>>>
>>> + lvds0: lvds@feb90000 {
>>> + compatible = "renesas,lvds-r8a77995";
>>> + reg = <0 0xfeb90000 0 0x20>;
>>> + clocks = <&cpg CPG_MOD 727>;
>>> + status = "disabled";
>>
>> Missing resets, power-domains
>
> Both lvds0, and lvds1 are actually the same 'module' (just different mappings
> within the same IP block).
>
> That means they will share clock, reset etc.
>
> Will specifying the resets cause one to affect the other unduly?
>
> like if one becomes un-used, will it power down? or is it refcounted, and will
> gracefully handle both device nodes from the same device?
>
> I'm already fairly sure clocks is fine for this - but it's just the reset's that
> worry me. If the reset module tries to reset lvds0 for some reason - would lvds1
> get reset as a side effect ...
Clocks handle sharing fine, they do refcounting.
For resets, there is some sharing/exclusive support, but we don't have that
much experience with resets as we don't use it that much yet (USB uses it).
So for now it's mostly about "describing the hardware".
Triggering a reset will indeed reset all modules using the same reset.
The same is true for e.g. PWM.
> Or perhaps that won't ever happen with an :
>
> power-domains = <&sysc R8A77995_PD_ALWAYS_ON>;
> resets = <&cpg 727>;
The power-domains property is related to power management (in this case
clock domain only, as it's the "always-on" domain), not reset.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Hi Kieran,
Thank you for the patch.
On Thursday, 15 February 2018 10:38:20 EET Kieran Bingham wrote:
> From: Kieran Bingham <[email protected]>
>
> The r8a77995 D3 platform has 2 LVDS channels connected to the DU.
>
> Signed-off-by: Kieran Bingham <[email protected]>
> ---
> arch/arm64/boot/dts/renesas/r8a77995.dtsi | 52 ++++++++++++++++++++++++++++
> 1 file changed, 52 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/renesas/r8a77995.dtsi
> b/arch/arm64/boot/dts/renesas/r8a77995.dtsi index
> a57d5fecf79c..7851e37cfff1 100644
> --- a/arch/arm64/boot/dts/renesas/r8a77995.dtsi
> +++ b/arch/arm64/boot/dts/renesas/r8a77995.dtsi
> @@ -773,18 +773,70 @@
> port@1 {
> reg = <1>;
> du_out_lvds0: endpoint {
> + remote-endpoint = <&du_out_lvds0>;
> };
> };
>
> port@2 {
> reg = <2>;
> du_out_lvds1: endpoint {
> + remote-endpoint = <&du_out_lvds1>;
It's interesting how the DU ports reference themselves :-)
> };
> };
> };
> };
> };
>
> + lvds0: lvds@feb90000 {
> + compatible = "renesas,lvds-r8a77995";
> + reg = <0 0xfeb90000 0 0x20>;
> + clocks = <&cpg CPG_MOD 727>;
> + status = "disabled";
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@0 {
> + reg = <0>;
> + lvds0_in: endpoint {
> + remote-endpoint = <&du_out_lvds0>;
> + };
> + };
> +
> + port@1 {
> + reg = <1>;
> + lvds0_out: endpoint {
> + };
> + };
> + };
> + };
> +
> + lvds1: lvds@feb90100 {
> + compatible = "renesas,lvds-r8a77995";
> + reg = <0 0xfeb90100 0 0x20>;
> + clocks = <&cpg CPG_MOD 727>;
> + status = "disabled";
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@0 {
> + reg = <0>;
> + lvds1_in: endpoint {
> + remote-endpoint = <&du_out_lvds1>;
> + };
> + };
> +
> + port@1 {
> + reg = <1>;
> + lvds1_out: endpoint {
> + };
> + };
> + };
> + };
> +
> timer {
> compatible = "arm,armv8-timer";
> interrupts-extended = <&gic GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(1) |
> IRQ_TYPE_LEVEL_LOW)>,
--
Regards,
Laurent Pinchart
On 15/02/18 14:10, Laurent Pinchart wrote:
> Hi Kieran,
>
> Thank you for the patch.
>
> On Thursday, 15 February 2018 10:38:20 EET Kieran Bingham wrote:
>> From: Kieran Bingham <[email protected]>
>>
>> The r8a77995 D3 platform has 2 LVDS channels connected to the DU.
>>
>> Signed-off-by: Kieran Bingham <[email protected]>
>> ---
>> arch/arm64/boot/dts/renesas/r8a77995.dtsi | 52 ++++++++++++++++++++++++++++
>> 1 file changed, 52 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/renesas/r8a77995.dtsi
>> b/arch/arm64/boot/dts/renesas/r8a77995.dtsi index
>> a57d5fecf79c..7851e37cfff1 100644
>> --- a/arch/arm64/boot/dts/renesas/r8a77995.dtsi
>> +++ b/arch/arm64/boot/dts/renesas/r8a77995.dtsi
>> @@ -773,18 +773,70 @@
>> port@1 {
>> reg = <1>;
>> du_out_lvds0: endpoint {
>> + remote-endpoint = <&du_out_lvds0>;
>> };
>> };
>>
>> port@2 {
>> reg = <2>;
>> du_out_lvds1: endpoint {
>> + remote-endpoint = <&du_out_lvds1>;
>
> It's interesting how the DU ports reference themselves :-)
Ah, yes - that's ... that's a feature...
--
Kieran
>> };
>> };
>> };
>> };
>> };
>>
>> + lvds0: lvds@feb90000 {
>> + compatible = "renesas,lvds-r8a77995";
>> + reg = <0 0xfeb90000 0 0x20>;
>> + clocks = <&cpg CPG_MOD 727>;
>> + status = "disabled";
>> +
>> + ports {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> +
>> + port@0 {
>> + reg = <0>;
>> + lvds0_in: endpoint {
>> + remote-endpoint = <&du_out_lvds0>;
>> + };
>> + };
>> +
>> + port@1 {
>> + reg = <1>;
>> + lvds0_out: endpoint {
>> + };
>> + };
>> + };
>> + };
>> +
>> + lvds1: lvds@feb90100 {
>> + compatible = "renesas,lvds-r8a77995";
>> + reg = <0 0xfeb90100 0 0x20>;
>> + clocks = <&cpg CPG_MOD 727>;
>> + status = "disabled";
>> +
>> + ports {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> +
>> + port@0 {
>> + reg = <0>;
>> + lvds1_in: endpoint {
>> + remote-endpoint = <&du_out_lvds1>;
>> + };
>> + };
>> +
>> + port@1 {
>> + reg = <1>;
>> + lvds1_out: endpoint {
>> + };
>> + };
>> + };
>> + };
>> +
>> timer {
>> compatible = "arm,armv8-timer";
>> interrupts-extended = <&gic GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(1) |
>> IRQ_TYPE_LEVEL_LOW)>,
>
On Thu, Feb 15, 2018 at 08:38:20AM +0000, Kieran Bingham wrote:
> From: Kieran Bingham <[email protected]>
>
> The r8a77995 D3 platform has 2 LVDS channels connected to the DU.
>
> Signed-off-by: Kieran Bingham <[email protected]>
It looks like there has been some review of this patch that needs
addressing, I've marked it as "Changes Requested".