* [PATCH v2 1/4] dt-bindings: drm/bridge: adv7511: Add regulator bindings
2016-09-23 9:20 ` [PATCH v2 0/4] arm64: apq8016-sbc: Enable regulators for ADV7533 Archit Taneja
@ 2016-09-23 9:20 ` Archit Taneja
2016-09-23 22:31 ` Rob Herring
[not found] ` <1474622430-6704-2-git-send-email-architt-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
2016-09-23 9:20 ` [PATCH v2 2/4] drm/bridge: adv7511: Initialize regulators Archit Taneja
` (3 subsequent siblings)
4 siblings, 2 replies; 52+ messages in thread
From: Archit Taneja @ 2016-09-23 9:20 UTC (permalink / raw)
To: andy.gross, laurent.pinchart
Cc: linux-arm-msm, robh, Archit Taneja, devicetree
Add the regulator supply properties needed by ADV7511 and ADV7533. The
regulator names are named after the pin names, except for DVDD_3V on
ADV7511, which is left v3p3 to have the same name as it's on ADV7533.
ADV7511 has a BGVDD pin for which the voltage to be configured isn't
clear. This needs to be updated in the document later.
The regulators are specified as optional properties since there can
be boards which have a fixed supply directly routed to the pins, and
these may not be modelled as regulator supplies.
Cc: Rob Herring <robh@kernel.org>
Cc: devicetree@vger.kernel.org
Signed-off-by: Archit Taneja <architt@codeaurora.org>
---
v2:
- Specify regulator supplies for ADV7511 too.
- Have separate supply entries for AVDD, DVDD, PVDD, A2VDD pins.
- Add additional BGVDD supply for ADV7511.
.../devicetree/bindings/display/bridge/adi,adv7511.txt | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
index 6532a59..66e6bae 100644
--- a/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
+++ b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
@@ -56,6 +56,20 @@ Optional properties:
- adi,disable-timing-generator: Only for ADV7533. Disables the internal timing
generator. The chip will rely on the sync signals in the DSI data lanes,
rather than generate its own timings for HDMI output.
+- avdd-supply: A 1.8V supply that powers up the AVDD pin on the chip.
+- dvdd-supply: A 1.8V supply that powers up the DVDD pin on the chip.
+- pvdd-supply: A 1.8V supply that powers up the PVDD pin on the chip.
+- v3p3-supply: A 3.3V supply that powers up the pin called DVDD_3V on
+ ADV7511 and V3P3 on ADV7533.
+
+ADV7511 specific supplies:
+- bgvdd-supply: Powers the BGVDD pin. The voltage isn't clear from
+ available datasheets.
+
+ADV7533 specific supplies:
+- a2vdd-supply: 1.8V supply that powers up the A2VDD pin on the chip.
+- v1p2-supply: A supply that powers up the V1P2 pin on the chip. It can be
+ either 1.2V or 1.8V.
Required nodes:
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
^ permalink raw reply related [flat|nested] 52+ messages in thread
* Re: [PATCH v2 1/4] dt-bindings: drm/bridge: adv7511: Add regulator bindings
2016-09-23 9:20 ` [PATCH v2 1/4] dt-bindings: drm/bridge: adv7511: Add regulator bindings Archit Taneja
@ 2016-09-23 22:31 ` Rob Herring
[not found] ` <1474622430-6704-2-git-send-email-architt-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
1 sibling, 0 replies; 52+ messages in thread
From: Rob Herring @ 2016-09-23 22:31 UTC (permalink / raw)
To: Archit Taneja; +Cc: andy.gross, laurent.pinchart, linux-arm-msm, devicetree
On Fri, Sep 23, 2016 at 02:50:27PM +0530, Archit Taneja wrote:
> Add the regulator supply properties needed by ADV7511 and ADV7533. The
> regulator names are named after the pin names, except for DVDD_3V on
> ADV7511, which is left v3p3 to have the same name as it's on ADV7533.
>
> ADV7511 has a BGVDD pin for which the voltage to be configured isn't
> clear. This needs to be updated in the document later.
>
> The regulators are specified as optional properties since there can
> be boards which have a fixed supply directly routed to the pins, and
> these may not be modelled as regulator supplies.
>
> Cc: Rob Herring <robh@kernel.org>
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Archit Taneja <architt@codeaurora.org>
> ---
> v2:
> - Specify regulator supplies for ADV7511 too.
> - Have separate supply entries for AVDD, DVDD, PVDD, A2VDD pins.
> - Add additional BGVDD supply for ADV7511.
>
> .../devicetree/bindings/display/bridge/adi,adv7511.txt | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
Acked-by: Rob Herring <robh@kernel.org>
^ permalink raw reply [flat|nested] 52+ messages in thread
[parent not found: <1474622430-6704-2-git-send-email-architt-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>]
* Re: [PATCH v2 1/4] dt-bindings: drm/bridge: adv7511: Add regulator bindings
[not found] ` <1474622430-6704-2-git-send-email-architt-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
@ 2016-09-25 17:28 ` Laurent Pinchart
2016-09-26 6:40 ` Archit Taneja
0 siblings, 1 reply; 52+ messages in thread
From: Laurent Pinchart @ 2016-09-25 17:28 UTC (permalink / raw)
To: Archit Taneja
Cc: andy.gross-QSEj5FYQhm4dnm+yROfE0A,
linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
robh-DgEjT+Ai2ygdnm+yROfE0A, devicetree-u79uwXL29TY76Z2rM5mHXA
Hi Archit,
Thank you for the patch.
On Friday 23 Sep 2016 14:50:27 Archit Taneja wrote:
> Add the regulator supply properties needed by ADV7511 and ADV7533. The
> regulator names are named after the pin names, except for DVDD_3V on
> ADV7511, which is left v3p3 to have the same name as it's on ADV7533.
>
> ADV7511 has a BGVDD pin for which the voltage to be configured isn't
> clear. This needs to be updated in the document later.
>
> The regulators are specified as optional properties since there can
> be boards which have a fixed supply directly routed to the pins, and
> these may not be modelled as regulator supplies.
>
> Cc: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Signed-off-by: Archit Taneja <architt-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
> ---
> v2:
> - Specify regulator supplies for ADV7511 too.
> - Have separate supply entries for AVDD, DVDD, PVDD, A2VDD pins.
> - Add additional BGVDD supply for ADV7511.
>
> .../devicetree/bindings/display/bridge/adi,adv7511.txt | 14 +++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git
> a/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
> b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt index
> 6532a59..66e6bae 100644
> --- a/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
> +++ b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
> @@ -56,6 +56,20 @@ Optional properties:
> - adi,disable-timing-generator: Only for ADV7533. Disables the internal
> timing generator. The chip will rely on the sync signals in the DSI data
> lanes, rather than generate its own timings for HDMI output.
> +- avdd-supply: A 1.8V supply that powers up the AVDD pin on the chip.
> +- dvdd-supply: A 1.8V supply that powers up the DVDD pin on the chip.
> +- pvdd-supply: A 1.8V supply that powers up the PVDD pin on the chip.
> +- v3p3-supply: A 3.3V supply that powers up the pin called DVDD_3V on
> + ADV7511 and V3P3 on ADV7533.
> +
> +ADV7511 specific supplies:
> +- bgvdd-supply: Powers the BGVDD pin. The voltage isn't clear from
> + available datasheets.
It's a 1.8V supply. The ADV7511 hardware user's guide recommends tying it to
the PVDD supply, so I'm not sure we need a separate supply in DT.
Similarly, the user's guide recommends supplying the AVDD, DVDD and PVDD pins
with a single 1.8V LDO and three independent filters. We could thus possibly
combine them into a single 1.8V supply.
> +ADV7533 specific supplies:
> +- a2vdd-supply: 1.8V supply that powers up the A2VDD pin on the chip.
> +- v1p2-supply: A supply that powers up the V1P2 pin on the chip. It can be
> + either 1.2V or 1.8V.
>
> Required nodes:
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH v2 1/4] dt-bindings: drm/bridge: adv7511: Add regulator bindings
2016-09-25 17:28 ` Laurent Pinchart
@ 2016-09-26 6:40 ` Archit Taneja
2016-09-26 8:12 ` Laurent Pinchart
0 siblings, 1 reply; 52+ messages in thread
From: Archit Taneja @ 2016-09-26 6:40 UTC (permalink / raw)
To: Laurent Pinchart
Cc: andy.gross-QSEj5FYQhm4dnm+yROfE0A,
linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
robh-DgEjT+Ai2ygdnm+yROfE0A, devicetree-u79uwXL29TY76Z2rM5mHXA
Hi Laurent,
On 9/25/2016 10:58 PM, Laurent Pinchart wrote:
> Hi Archit,
>
> Thank you for the patch.
>
> On Friday 23 Sep 2016 14:50:27 Archit Taneja wrote:
>> Add the regulator supply properties needed by ADV7511 and ADV7533. The
>> regulator names are named after the pin names, except for DVDD_3V on
>> ADV7511, which is left v3p3 to have the same name as it's on ADV7533.
>>
>> ADV7511 has a BGVDD pin for which the voltage to be configured isn't
>> clear. This needs to be updated in the document later.
>>
>> The regulators are specified as optional properties since there can
>> be boards which have a fixed supply directly routed to the pins, and
>> these may not be modelled as regulator supplies.
>>
>> Cc: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> Signed-off-by: Archit Taneja <architt-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
>> ---
>> v2:
>> - Specify regulator supplies for ADV7511 too.
>> - Have separate supply entries for AVDD, DVDD, PVDD, A2VDD pins.
>> - Add additional BGVDD supply for ADV7511.
>>
>> .../devicetree/bindings/display/bridge/adi,adv7511.txt | 14 +++++++++++
>> 1 file changed, 14 insertions(+)
>>
>> diff --git
>> a/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
>> b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt index
>> 6532a59..66e6bae 100644
>> --- a/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
>> +++ b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
>> @@ -56,6 +56,20 @@ Optional properties:
>> - adi,disable-timing-generator: Only for ADV7533. Disables the internal
>> timing generator. The chip will rely on the sync signals in the DSI data
>> lanes, rather than generate its own timings for HDMI output.
>> +- avdd-supply: A 1.8V supply that powers up the AVDD pin on the chip.
>> +- dvdd-supply: A 1.8V supply that powers up the DVDD pin on the chip.
>> +- pvdd-supply: A 1.8V supply that powers up the PVDD pin on the chip.
>> +- v3p3-supply: A 3.3V supply that powers up the pin called DVDD_3V on
>> + ADV7511 and V3P3 on ADV7533.
>> +
>> +ADV7511 specific supplies:
>> +- bgvdd-supply: Powers the BGVDD pin. The voltage isn't clear from
>> + available datasheets.
>
> It's a 1.8V supply. The ADV7511 hardware user's guide recommends tying it to
> the PVDD supply, so I'm not sure we need a separate supply in DT.
Thanks for providing the info. I'll update it in the doc.
>
> Similarly, the user's guide recommends supplying the AVDD, DVDD and PVDD pins
> with a single 1.8V LDO and three independent filters. We could thus possibly
> combine them into a single 1.8V supply.
>
I'd originally specified only one AVDD supply for all the 1.8 V
supplies, but Rob had recommended using separate supplies for each pin.
I guess it doesn't do much harm specifying separate supplies, a
board can just provide avdd-supply, and the rest would be assumed dummy.
It could also help describe boards which may not have followed the
user guide's recommendation, but I agree that's probably a rare
scenario.
Is it okay if we stick with multiple supplies as it is in this version?
Thanks for the review.
Archit
>> +ADV7533 specific supplies:
>> +- a2vdd-supply: 1.8V supply that powers up the A2VDD pin on the chip.
>> +- v1p2-supply: A supply that powers up the V1P2 pin on the chip. It can be
>> + either 1.2V or 1.8V.
>>
>> Required nodes:
>
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH v2 1/4] dt-bindings: drm/bridge: adv7511: Add regulator bindings
2016-09-26 6:40 ` Archit Taneja
@ 2016-09-26 8:12 ` Laurent Pinchart
2016-10-14 5:40 ` Archit Taneja
0 siblings, 1 reply; 52+ messages in thread
From: Laurent Pinchart @ 2016-09-26 8:12 UTC (permalink / raw)
To: Archit Taneja; +Cc: andy.gross, linux-arm-msm, robh, devicetree
Hi Archit,
On Monday 26 Sep 2016 12:10:11 Archit Taneja wrote:
> On 9/25/2016 10:58 PM, Laurent Pinchart wrote:
> > On Friday 23 Sep 2016 14:50:27 Archit Taneja wrote:
> >> Add the regulator supply properties needed by ADV7511 and ADV7533. The
> >> regulator names are named after the pin names, except for DVDD_3V on
> >> ADV7511, which is left v3p3 to have the same name as it's on ADV7533.
> >>
> >> ADV7511 has a BGVDD pin for which the voltage to be configured isn't
> >> clear. This needs to be updated in the document later.
> >>
> >> The regulators are specified as optional properties since there can
> >> be boards which have a fixed supply directly routed to the pins, and
> >> these may not be modelled as regulator supplies.
> >>
> >> Cc: Rob Herring <robh@kernel.org>
> >> Cc: devicetree@vger.kernel.org
> >> Signed-off-by: Archit Taneja <architt@codeaurora.org>
> >> ---
> >> v2:
> >> - Specify regulator supplies for ADV7511 too.
> >> - Have separate supply entries for AVDD, DVDD, PVDD, A2VDD pins.
> >> - Add additional BGVDD supply for ADV7511.
> >>
> >> .../devicetree/bindings/display/bridge/adi,adv7511.txt | 14 +++++++
> >> 1 file changed, 14 insertions(+)
> >>
> >> diff --git
> >> a/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
> >> b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt index
> >> 6532a59..66e6bae 100644
> >> --- a/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
> >> +++ b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
> >>
> >> @@ -56,6 +56,20 @@ Optional properties:
> >> - adi,disable-timing-generator: Only for ADV7533. Disables the internal
> >>
> >> timing generator. The chip will rely on the sync signals in the DSI data
> >> lanes, rather than generate its own timings for HDMI output.
> >> +- avdd-supply: A 1.8V supply that powers up the AVDD pin on the chip.
> >> +- dvdd-supply: A 1.8V supply that powers up the DVDD pin on the chip.
> >> +- pvdd-supply: A 1.8V supply that powers up the PVDD pin on the chip.
> >> +- v3p3-supply: A 3.3V supply that powers up the pin called DVDD_3V on
> >> + ADV7511 and V3P3 on ADV7533.
> >> +
> >> +ADV7511 specific supplies:
> >> +- bgvdd-supply: Powers the BGVDD pin. The voltage isn't clear from
> >> + available datasheets.
> >
> > It's a 1.8V supply. The ADV7511 hardware user's guide recommends tying it
> > to the PVDD supply, so I'm not sure we need a separate supply in DT.
>
> Thanks for providing the info. I'll update it in the doc.
>
> > Similarly, the user's guide recommends supplying the AVDD, DVDD and PVDD
> > pins with a single 1.8V LDO and three independent filters. We could thus
> > possibly combine them into a single 1.8V supply.
>
> I'd originally specified only one AVDD supply for all the 1.8 V
> supplies, but Rob had recommended using separate supplies for each pin.
>
> I guess it doesn't do much harm specifying separate supplies, a
> board can just provide avdd-supply, and the rest would be assumed dummy.
DT should use the same regulator phandle for all power supplies in that case,
not specify a single one.
> It could also help describe boards which may not have followed the
> user guide's recommendation, but I agree that's probably a rare
> scenario.
>
> Is it okay if we stick with multiple supplies as it is in this version?
It seems overkill to me but I can live with that. I'd remove the bgvdd supply
though.
Rob, what's your opinion on this ? Is there a specific reason why you prefer
splitting the 1.8V supply in one supply per pin, when the hardware user's
guide recommend powering them from the same LDO ?
> >> +ADV7533 specific supplies:
> >> +- a2vdd-supply: 1.8V supply that powers up the A2VDD pin on the chip.
> >> +- v1p2-supply: A supply that powers up the V1P2 pin on the chip. It can
> >> be
> >> + either 1.2V or 1.8V.
> >>
> >> Required nodes:
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH v2 1/4] dt-bindings: drm/bridge: adv7511: Add regulator bindings
2016-09-26 8:12 ` Laurent Pinchart
@ 2016-10-14 5:40 ` Archit Taneja
2016-11-10 1:34 ` Rob Herring
0 siblings, 1 reply; 52+ messages in thread
From: Archit Taneja @ 2016-10-14 5:40 UTC (permalink / raw)
To: Laurent Pinchart, robh; +Cc: andy.gross, linux-arm-msm, devicetree
Hi Rob,
On 09/26/2016 01:42 PM, Laurent Pinchart wrote:
> Hi Archit,
>
> On Monday 26 Sep 2016 12:10:11 Archit Taneja wrote:
>> On 9/25/2016 10:58 PM, Laurent Pinchart wrote:
>>> On Friday 23 Sep 2016 14:50:27 Archit Taneja wrote:
>>>> Add the regulator supply properties needed by ADV7511 and ADV7533. The
>>>> regulator names are named after the pin names, except for DVDD_3V on
>>>> ADV7511, which is left v3p3 to have the same name as it's on ADV7533.
>>>>
>>>> ADV7511 has a BGVDD pin for which the voltage to be configured isn't
>>>> clear. This needs to be updated in the document later.
>>>>
>>>> The regulators are specified as optional properties since there can
>>>> be boards which have a fixed supply directly routed to the pins, and
>>>> these may not be modelled as regulator supplies.
>>>>
>>>> Cc: Rob Herring <robh@kernel.org>
>>>> Cc: devicetree@vger.kernel.org
>>>> Signed-off-by: Archit Taneja <architt@codeaurora.org>
>>>> ---
>>>> v2:
>>>> - Specify regulator supplies for ADV7511 too.
>>>> - Have separate supply entries for AVDD, DVDD, PVDD, A2VDD pins.
>>>> - Add additional BGVDD supply for ADV7511.
>>>>
>>>> .../devicetree/bindings/display/bridge/adi,adv7511.txt | 14 +++++++
>>>> 1 file changed, 14 insertions(+)
>>>>
>>>> diff --git
>>>> a/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
>>>> b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt index
>>>> 6532a59..66e6bae 100644
>>>> --- a/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
>>>> +++ b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
>>>>
>>>> @@ -56,6 +56,20 @@ Optional properties:
>>>> - adi,disable-timing-generator: Only for ADV7533. Disables the internal
>>>>
>>>> timing generator. The chip will rely on the sync signals in the DSI data
>>>> lanes, rather than generate its own timings for HDMI output.
>>>> +- avdd-supply: A 1.8V supply that powers up the AVDD pin on the chip.
>>>> +- dvdd-supply: A 1.8V supply that powers up the DVDD pin on the chip.
>>>> +- pvdd-supply: A 1.8V supply that powers up the PVDD pin on the chip.
>>>> +- v3p3-supply: A 3.3V supply that powers up the pin called DVDD_3V on
>>>> + ADV7511 and V3P3 on ADV7533.
>>>> +
>>>> +ADV7511 specific supplies:
>>>> +- bgvdd-supply: Powers the BGVDD pin. The voltage isn't clear from
>>>> + available datasheets.
>>>
>>> It's a 1.8V supply. The ADV7511 hardware user's guide recommends tying it
>>> to the PVDD supply, so I'm not sure we need a separate supply in DT.
>>
>> Thanks for providing the info. I'll update it in the doc.
>>
>>> Similarly, the user's guide recommends supplying the AVDD, DVDD and PVDD
>>> pins with a single 1.8V LDO and three independent filters. We could thus
>>> possibly combine them into a single 1.8V supply.
>>
>> I'd originally specified only one AVDD supply for all the 1.8 V
>> supplies, but Rob had recommended using separate supplies for each pin.
>>
>> I guess it doesn't do much harm specifying separate supplies, a
>> board can just provide avdd-supply, and the rest would be assumed dummy.
>
> DT should use the same regulator phandle for all power supplies in that case,
> not specify a single one.
>
>> It could also help describe boards which may not have followed the
>> user guide's recommendation, but I agree that's probably a rare
>> scenario.
>>
>> Is it okay if we stick with multiple supplies as it is in this version?
>
> It seems overkill to me but I can live with that. I'd remove the bgvdd supply
> though.
>
> Rob, what's your opinion on this ? Is there a specific reason why you prefer
> splitting the 1.8V supply in one supply per pin, when the hardware user's
> guide recommend powering them from the same LDO ?
Could you suggest us which way to go?
Thanks,
Archit
>
>>>> +ADV7533 specific supplies:
>>>> +- a2vdd-supply: 1.8V supply that powers up the A2VDD pin on the chip.
>>>> +- v1p2-supply: A supply that powers up the V1P2 pin on the chip. It can
>>>> be
>>>> + either 1.2V or 1.8V.
>>>>
>>>> Required nodes:
>
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH v2 1/4] dt-bindings: drm/bridge: adv7511: Add regulator bindings
2016-10-14 5:40 ` Archit Taneja
@ 2016-11-10 1:34 ` Rob Herring
[not found] ` <CAL_Jsq+KMPoyWvh1U3OKL6pX8D4QByqVFgcMks8myDfZEYeFCg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
0 siblings, 1 reply; 52+ messages in thread
From: Rob Herring @ 2016-11-10 1:34 UTC (permalink / raw)
To: Archit Taneja; +Cc: Laurent Pinchart, Andy Gross, linux-arm-msm, devicetree
On Fri, Oct 14, 2016 at 12:40 AM, Archit Taneja <architt@codeaurora.org> wrote:
> Hi Rob,
>
>
> On 09/26/2016 01:42 PM, Laurent Pinchart wrote:
>>
>> Hi Archit,
>>
>> On Monday 26 Sep 2016 12:10:11 Archit Taneja wrote:
>>>
>>> On 9/25/2016 10:58 PM, Laurent Pinchart wrote:
>>>>
>>>> On Friday 23 Sep 2016 14:50:27 Archit Taneja wrote:
>>>>>
>>>>> Add the regulator supply properties needed by ADV7511 and ADV7533. The
>>>>> regulator names are named after the pin names, except for DVDD_3V on
>>>>> ADV7511, which is left v3p3 to have the same name as it's on ADV7533.
>>>>>
>>>>> ADV7511 has a BGVDD pin for which the voltage to be configured isn't
>>>>> clear. This needs to be updated in the document later.
>>>>>
>>>>> The regulators are specified as optional properties since there can
>>>>> be boards which have a fixed supply directly routed to the pins, and
>>>>> these may not be modelled as regulator supplies.
>>>>>
>>>>> Cc: Rob Herring <robh@kernel.org>
>>>>> Cc: devicetree@vger.kernel.org
>>>>> Signed-off-by: Archit Taneja <architt@codeaurora.org>
>>>>> ---
>>>>> v2:
>>>>> - Specify regulator supplies for ADV7511 too.
>>>>> - Have separate supply entries for AVDD, DVDD, PVDD, A2VDD pins.
>>>>> - Add additional BGVDD supply for ADV7511.
>>>>>
>>>>> .../devicetree/bindings/display/bridge/adi,adv7511.txt | 14
>>>>> +++++++
>>>>> 1 file changed, 14 insertions(+)
>>>>>
>>>>> diff --git
>>>>> a/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
>>>>> b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
>>>>> index
>>>>> 6532a59..66e6bae 100644
>>>>> --- a/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
>>>>> +++ b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
>>>>>
>>>>> @@ -56,6 +56,20 @@ Optional properties:
>>>>> - adi,disable-timing-generator: Only for ADV7533. Disables the
>>>>> internal
>>>>>
>>>>> timing generator. The chip will rely on the sync signals in the DSI
>>>>> data
>>>>> lanes, rather than generate its own timings for HDMI output.
>>>>> +- avdd-supply: A 1.8V supply that powers up the AVDD pin on the chip.
>>>>> +- dvdd-supply: A 1.8V supply that powers up the DVDD pin on the chip.
>>>>> +- pvdd-supply: A 1.8V supply that powers up the PVDD pin on the chip.
>>>>> +- v3p3-supply: A 3.3V supply that powers up the pin called DVDD_3V on
>>>>> + ADV7511 and V3P3 on ADV7533.
>>>>> +
>>>>> +ADV7511 specific supplies:
>>>>> +- bgvdd-supply: Powers the BGVDD pin. The voltage isn't clear from
>>>>> + available datasheets.
>>>>
>>>>
>>>> It's a 1.8V supply. The ADV7511 hardware user's guide recommends tying
>>>> it
>>>> to the PVDD supply, so I'm not sure we need a separate supply in DT.
>>>
>>>
>>> Thanks for providing the info. I'll update it in the doc.
>>>
>>>> Similarly, the user's guide recommends supplying the AVDD, DVDD and PVDD
>>>> pins with a single 1.8V LDO and three independent filters. We could thus
>>>> possibly combine them into a single 1.8V supply.
>>>
>>>
>>> I'd originally specified only one AVDD supply for all the 1.8 V
>>> supplies, but Rob had recommended using separate supplies for each pin.
>>>
>>> I guess it doesn't do much harm specifying separate supplies, a
>>> board can just provide avdd-supply, and the rest would be assumed dummy.
>>
>>
>> DT should use the same regulator phandle for all power supplies in that
>> case,
>> not specify a single one.
>>
>>> It could also help describe boards which may not have followed the
>>> user guide's recommendation, but I agree that's probably a rare
>>> scenario.
>>>
>>> Is it okay if we stick with multiple supplies as it is in this version?
>>
>>
>> It seems overkill to me but I can live with that. I'd remove the bgvdd
>> supply
>> though.
>>
>> Rob, what's your opinion on this ? Is there a specific reason why you
>> prefer
>> splitting the 1.8V supply in one supply per pin, when the hardware user's
>> guide recommend powering them from the same LDO ?
>
>
> Could you suggest us which way to go?
If they are really intended to be the same supply, then combining them
is fine with me.
Rob
^ permalink raw reply [flat|nested] 52+ messages in thread
* [PATCH v2 2/4] drm/bridge: adv7511: Initialize regulators
2016-09-23 9:20 ` [PATCH v2 0/4] arm64: apq8016-sbc: Enable regulators for ADV7533 Archit Taneja
2016-09-23 9:20 ` [PATCH v2 1/4] dt-bindings: drm/bridge: adv7511: Add regulator bindings Archit Taneja
@ 2016-09-23 9:20 ` Archit Taneja
2016-09-25 17:35 ` Laurent Pinchart
2016-09-23 9:20 ` [PATCH v2 3/4] arm64: dts: apq8016-sbc: Add some missing regulator supplies for ADV7533 Archit Taneja
` (2 subsequent siblings)
4 siblings, 1 reply; 52+ messages in thread
From: Archit Taneja @ 2016-09-23 9:20 UTC (permalink / raw)
To: andy.gross, laurent.pinchart; +Cc: linux-arm-msm, dri-devel
Maintain a table of regulator names expect by ADV7511 and ADV7533.
Use regulator_bulk_* api to configure these.
Initialize and enable the regulators during probe itself. Controlling
these dynamically is left for later.
Cc: dri-devel@lists.freedesktop.org
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Archit Taneja <architt@codeaurora.org>
---
v2:
- Use regulator_bulk_* API to configure regulators as suggested by Laurent.
- Set up regulators for ADV7511 too.
drivers/gpu/drm/bridge/adv7511/adv7511.h | 4 ++
drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 85 +++++++++++++++++++++++++---
2 files changed, 80 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511.h b/drivers/gpu/drm/bridge/adv7511/adv7511.h
index 161c923..83ebdaa 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511.h
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511.h
@@ -12,6 +12,7 @@
#include <linux/hdmi.h>
#include <linux/i2c.h>
#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
#include <drm/drm_crtc_helper.h>
#include <drm/drm_mipi_dsi.h>
@@ -327,6 +328,9 @@ struct adv7511 {
struct gpio_desc *gpio_pd;
+ struct regulator_bulk_data *supplies;
+ int num_supplies;
+
/* ADV7533 DSI RX related params */
struct device_node *host_node;
struct mipi_dsi_device *dsi;
diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
index 8ed3906..f7e79ed 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
@@ -839,6 +839,57 @@ static struct drm_bridge_funcs adv7511_bridge_funcs = {
* Probe & remove
*/
+static const char * const adv7511_supply_names[] = {
+ "avdd",
+ "dvdd",
+ "pvdd",
+ "v3p3",
+ "bgvdd",
+};
+
+static const char * const adv7533_supply_names[] = {
+ "avdd",
+ "dvdd",
+ "pvdd",
+ "a2vdd",
+ "v3p3",
+ "v1p2",
+};
+
+static int adv7511_init_regulators(struct adv7511 *adv)
+{
+ struct device *dev = &adv->i2c_main->dev;
+ const char * const *supply_names;
+ int i, ret;
+
+ if (adv->type == ADV7511) {
+ supply_names = adv7511_supply_names;
+ adv->num_supplies = ARRAY_SIZE(adv7511_supply_names);
+ } else {
+ supply_names = adv7533_supply_names;
+ adv->num_supplies = ARRAY_SIZE(adv7533_supply_names);
+ }
+
+ adv->supplies = devm_kcalloc(dev, adv->num_supplies,
+ sizeof(*adv->supplies), GFP_KERNEL);
+ if (!adv->supplies)
+ return -ENOMEM;
+
+ for (i = 0; i < adv->num_supplies; i++)
+ adv->supplies[i].supply = supply_names[i];
+
+ ret = devm_regulator_bulk_get(dev, adv->num_supplies, adv->supplies);
+ if (ret)
+ return ret;
+
+ return regulator_bulk_enable(adv->num_supplies, adv->supplies);
+}
+
+static void adv7511_uninit_regulators(struct adv7511 *adv)
+{
+ regulator_bulk_disable(adv->num_supplies, adv->supplies);
+}
+
static int adv7511_parse_dt(struct device_node *np,
struct adv7511_link_config *config)
{
@@ -939,6 +990,7 @@ static int adv7511_probe(struct i2c_client *i2c, const struct i2c_device_id *id)
if (!adv7511)
return -ENOMEM;
+ adv7511->i2c_main = i2c;
adv7511->powered = false;
adv7511->status = connector_status_disconnected;
@@ -956,13 +1008,21 @@ static int adv7511_probe(struct i2c_client *i2c, const struct i2c_device_id *id)
if (ret)
return ret;
+ ret = adv7511_init_regulators(adv7511);
+ if (ret) {
+ dev_err(dev, "failed to init regulators\n");
+ return ret;
+ }
+
/*
* The power down GPIO is optional. If present, toggle it from active to
* inactive to wake up the encoder.
*/
adv7511->gpio_pd = devm_gpiod_get_optional(dev, "pd", GPIOD_OUT_HIGH);
- if (IS_ERR(adv7511->gpio_pd))
- return PTR_ERR(adv7511->gpio_pd);
+ if (IS_ERR(adv7511->gpio_pd)) {
+ ret = PTR_ERR(adv7511->gpio_pd);
+ goto uninit_regulators;
+ }
if (adv7511->gpio_pd) {
mdelay(5);
@@ -970,12 +1030,14 @@ static int adv7511_probe(struct i2c_client *i2c, const struct i2c_device_id *id)
}
adv7511->regmap = devm_regmap_init_i2c(i2c, &adv7511_regmap_config);
- if (IS_ERR(adv7511->regmap))
- return PTR_ERR(adv7511->regmap);
+ if (IS_ERR(adv7511->regmap)) {
+ ret = PTR_ERR(adv7511->regmap);
+ goto uninit_regulators;
+ }
ret = regmap_read(adv7511->regmap, ADV7511_REG_CHIP_REVISION, &val);
if (ret)
- return ret;
+ goto uninit_regulators;
dev_dbg(dev, "Rev. %d\n", val);
if (adv7511->type == ADV7511)
@@ -985,7 +1047,7 @@ static int adv7511_probe(struct i2c_client *i2c, const struct i2c_device_id *id)
else
ret = adv7533_patch_registers(adv7511);
if (ret)
- return ret;
+ goto uninit_regulators;
regmap_write(adv7511->regmap, ADV7511_REG_EDID_I2C_ADDR, edid_i2c_addr);
regmap_write(adv7511->regmap, ADV7511_REG_PACKET_I2C_ADDR,
@@ -995,10 +1057,11 @@ static int adv7511_probe(struct i2c_client *i2c, const struct i2c_device_id *id)
adv7511_packet_disable(adv7511, 0xffff);
- adv7511->i2c_main = i2c;
adv7511->i2c_edid = i2c_new_dummy(i2c->adapter, edid_i2c_addr >> 1);
- if (!adv7511->i2c_edid)
- return -ENOMEM;
+ if (!adv7511->i2c_edid) {
+ ret = -ENOMEM;
+ goto uninit_regulators;
+ }
if (adv7511->type == ADV7533) {
ret = adv7533_init_cec(adv7511);
@@ -1043,6 +1106,8 @@ err_unregister_cec:
adv7533_uninit_cec(adv7511);
err_i2c_unregister_edid:
i2c_unregister_device(adv7511->i2c_edid);
+uninit_regulators:
+ adv7511_uninit_regulators(adv7511);
return ret;
}
@@ -1056,6 +1121,8 @@ static int adv7511_remove(struct i2c_client *i2c)
adv7533_uninit_cec(adv7511);
}
+ adv7511_uninit_regulators(adv7511);
+
drm_bridge_remove(&adv7511->bridge);
i2c_unregister_device(adv7511->i2c_edid);
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 52+ messages in thread
* Re: [PATCH v2 2/4] drm/bridge: adv7511: Initialize regulators
2016-09-23 9:20 ` [PATCH v2 2/4] drm/bridge: adv7511: Initialize regulators Archit Taneja
@ 2016-09-25 17:35 ` Laurent Pinchart
2016-09-26 6:40 ` Archit Taneja
0 siblings, 1 reply; 52+ messages in thread
From: Laurent Pinchart @ 2016-09-25 17:35 UTC (permalink / raw)
To: Archit Taneja; +Cc: andy.gross, linux-arm-msm, dri-devel
Hi Archit,
Thank you for the patch.
On Friday 23 Sep 2016 14:50:28 Archit Taneja wrote:
> Maintain a table of regulator names expect by ADV7511 and ADV7533.
> Use regulator_bulk_* api to configure these.
>
> Initialize and enable the regulators during probe itself. Controlling
> these dynamically is left for later.
>
> Cc: dri-devel@lists.freedesktop.org
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Archit Taneja <architt@codeaurora.org>
> ---
> v2:
> - Use regulator_bulk_* API to configure regulators as suggested by Laurent.
> - Set up regulators for ADV7511 too.
>
> drivers/gpu/drm/bridge/adv7511/adv7511.h | 4 ++
> drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 85 ++++++++++++++++++++++---
> 2 files changed, 80 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511.h
> b/drivers/gpu/drm/bridge/adv7511/adv7511.h index 161c923..83ebdaa 100644
> --- a/drivers/gpu/drm/bridge/adv7511/adv7511.h
> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511.h
> @@ -12,6 +12,7 @@
> #include <linux/hdmi.h>
> #include <linux/i2c.h>
> #include <linux/regmap.h>
> +#include <linux/regulator/consumer.h>
>
> #include <drm/drm_crtc_helper.h>
> #include <drm/drm_mipi_dsi.h>
> @@ -327,6 +328,9 @@ struct adv7511 {
>
> struct gpio_desc *gpio_pd;
>
> + struct regulator_bulk_data *supplies;
> + int num_supplies;
num_supplies is always positive, you can make it an unsigned int.
> +
> /* ADV7533 DSI RX related params */
> struct device_node *host_node;
> struct mipi_dsi_device *dsi;
> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c index 8ed3906..f7e79ed
> 100644
> --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> @@ -839,6 +839,57 @@ static struct drm_bridge_funcs adv7511_bridge_funcs = {
> * Probe & remove
> */
>
> +static const char * const adv7511_supply_names[] = {
> + "avdd",
> + "dvdd",
> + "pvdd",
> + "v3p3",
> + "bgvdd",
> +};
> +
> +static const char * const adv7533_supply_names[] = {
> + "avdd",
> + "dvdd",
> + "pvdd",
> + "a2vdd",
> + "v3p3",
> + "v1p2",
> +};
> +
> +static int adv7511_init_regulators(struct adv7511 *adv)
> +{
> + struct device *dev = &adv->i2c_main->dev;
> + const char * const *supply_names;
> + int i, ret;
i only takes positive values, you can make it an unsigned int.
Apart from that,
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> +
> + if (adv->type == ADV7511) {
> + supply_names = adv7511_supply_names;
> + adv->num_supplies = ARRAY_SIZE(adv7511_supply_names);
> + } else {
> + supply_names = adv7533_supply_names;
> + adv->num_supplies = ARRAY_SIZE(adv7533_supply_names);
> + }
> +
> + adv->supplies = devm_kcalloc(dev, adv->num_supplies,
> + sizeof(*adv->supplies), GFP_KERNEL);
> + if (!adv->supplies)
> + return -ENOMEM;
> +
> + for (i = 0; i < adv->num_supplies; i++)
> + adv->supplies[i].supply = supply_names[i];
> +
> + ret = devm_regulator_bulk_get(dev, adv->num_supplies, adv->supplies);
> + if (ret)
> + return ret;
> +
> + return regulator_bulk_enable(adv->num_supplies, adv->supplies);
> +}
> +
> +static void adv7511_uninit_regulators(struct adv7511 *adv)
> +{
> + regulator_bulk_disable(adv->num_supplies, adv->supplies);
> +}
> +
> static int adv7511_parse_dt(struct device_node *np,
> struct adv7511_link_config *config)
> {
> @@ -939,6 +990,7 @@ static int adv7511_probe(struct i2c_client *i2c, const
> struct i2c_device_id *id) if (!adv7511)
> return -ENOMEM;
>
> + adv7511->i2c_main = i2c;
> adv7511->powered = false;
> adv7511->status = connector_status_disconnected;
>
> @@ -956,13 +1008,21 @@ static int adv7511_probe(struct i2c_client *i2c,
> const struct i2c_device_id *id)
> if (ret)
> return ret;
>
> + ret = adv7511_init_regulators(adv7511);
> + if (ret) {
> + dev_err(dev, "failed to init regulators\n");
> + return ret;
> + }
> +
> /*
> * The power down GPIO is optional. If present, toggle it from active
> to
> * inactive to wake up the encoder.
> */
> adv7511->gpio_pd = devm_gpiod_get_optional(dev, "pd", GPIOD_OUT_HIGH);
> - if (IS_ERR(adv7511->gpio_pd))
> - return PTR_ERR(adv7511->gpio_pd);
> + if (IS_ERR(adv7511->gpio_pd)) {
> + ret = PTR_ERR(adv7511->gpio_pd);
> + goto uninit_regulators;
> + }
>
> if (adv7511->gpio_pd) {
> mdelay(5);
> @@ -970,12 +1030,14 @@ static int adv7511_probe(struct i2c_client *i2c,
> const struct i2c_device_id *id) }
>
> adv7511->regmap = devm_regmap_init_i2c(i2c, &adv7511_regmap_config);
> - if (IS_ERR(adv7511->regmap))
> - return PTR_ERR(adv7511->regmap);
> + if (IS_ERR(adv7511->regmap)) {
> + ret = PTR_ERR(adv7511->regmap);
> + goto uninit_regulators;
> + }
>
> ret = regmap_read(adv7511->regmap, ADV7511_REG_CHIP_REVISION, &val);
> if (ret)
> - return ret;
> + goto uninit_regulators;
> dev_dbg(dev, "Rev. %d\n", val);
>
> if (adv7511->type == ADV7511)
> @@ -985,7 +1047,7 @@ static int adv7511_probe(struct i2c_client *i2c, const
> struct i2c_device_id *id) else
> ret = adv7533_patch_registers(adv7511);
> if (ret)
> - return ret;
> + goto uninit_regulators;
>
> regmap_write(adv7511->regmap, ADV7511_REG_EDID_I2C_ADDR,
edid_i2c_addr);
> regmap_write(adv7511->regmap, ADV7511_REG_PACKET_I2C_ADDR,
> @@ -995,10 +1057,11 @@ static int adv7511_probe(struct i2c_client *i2c,
> const struct i2c_device_id *id)
>
> adv7511_packet_disable(adv7511, 0xffff);
>
> - adv7511->i2c_main = i2c;
> adv7511->i2c_edid = i2c_new_dummy(i2c->adapter, edid_i2c_addr >> 1);
> - if (!adv7511->i2c_edid)
> - return -ENOMEM;
> + if (!adv7511->i2c_edid) {
> + ret = -ENOMEM;
> + goto uninit_regulators;
> + }
>
> if (adv7511->type == ADV7533) {
> ret = adv7533_init_cec(adv7511);
> @@ -1043,6 +1106,8 @@ err_unregister_cec:
> adv7533_uninit_cec(adv7511);
> err_i2c_unregister_edid:
> i2c_unregister_device(adv7511->i2c_edid);
> +uninit_regulators:
> + adv7511_uninit_regulators(adv7511);
>
> return ret;
> }
> @@ -1056,6 +1121,8 @@ static int adv7511_remove(struct i2c_client *i2c)
> adv7533_uninit_cec(adv7511);
> }
>
> + adv7511_uninit_regulators(adv7511);
> +
> drm_bridge_remove(&adv7511->bridge);
>
> i2c_unregister_device(adv7511->i2c_edid);
--
Regards,
Laurent Pinchart
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH v2 2/4] drm/bridge: adv7511: Initialize regulators
2016-09-25 17:35 ` Laurent Pinchart
@ 2016-09-26 6:40 ` Archit Taneja
0 siblings, 0 replies; 52+ messages in thread
From: Archit Taneja @ 2016-09-26 6:40 UTC (permalink / raw)
To: Laurent Pinchart; +Cc: andy.gross, linux-arm-msm, dri-devel
On 9/25/2016 11:05 PM, Laurent Pinchart wrote:
> Hi Archit,
>
> Thank you for the patch.
>
> On Friday 23 Sep 2016 14:50:28 Archit Taneja wrote:
>> Maintain a table of regulator names expect by ADV7511 and ADV7533.
>> Use regulator_bulk_* api to configure these.
>>
>> Initialize and enable the regulators during probe itself. Controlling
>> these dynamically is left for later.
>>
>> Cc: dri-devel@lists.freedesktop.org
>> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>> Signed-off-by: Archit Taneja <architt@codeaurora.org>
>> ---
>> v2:
>> - Use regulator_bulk_* API to configure regulators as suggested by Laurent.
>> - Set up regulators for ADV7511 too.
>>
>> drivers/gpu/drm/bridge/adv7511/adv7511.h | 4 ++
>> drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 85 ++++++++++++++++++++++---
>> 2 files changed, 80 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511.h
>> b/drivers/gpu/drm/bridge/adv7511/adv7511.h index 161c923..83ebdaa 100644
>> --- a/drivers/gpu/drm/bridge/adv7511/adv7511.h
>> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511.h
>> @@ -12,6 +12,7 @@
>> #include <linux/hdmi.h>
>> #include <linux/i2c.h>
>> #include <linux/regmap.h>
>> +#include <linux/regulator/consumer.h>
>>
>> #include <drm/drm_crtc_helper.h>
>> #include <drm/drm_mipi_dsi.h>
>> @@ -327,6 +328,9 @@ struct adv7511 {
>>
>> struct gpio_desc *gpio_pd;
>>
>> + struct regulator_bulk_data *supplies;
>> + int num_supplies;
>
> num_supplies is always positive, you can make it an unsigned int.
>
>> +
>> /* ADV7533 DSI RX related params */
>> struct device_node *host_node;
>> struct mipi_dsi_device *dsi;
>> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
>> b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c index 8ed3906..f7e79ed
>> 100644
>> --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
>> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
>> @@ -839,6 +839,57 @@ static struct drm_bridge_funcs adv7511_bridge_funcs = {
>> * Probe & remove
>> */
>>
>> +static const char * const adv7511_supply_names[] = {
>> + "avdd",
>> + "dvdd",
>> + "pvdd",
>> + "v3p3",
>> + "bgvdd",
>> +};
>> +
>> +static const char * const adv7533_supply_names[] = {
>> + "avdd",
>> + "dvdd",
>> + "pvdd",
>> + "a2vdd",
>> + "v3p3",
>> + "v1p2",
>> +};
>> +
>> +static int adv7511_init_regulators(struct adv7511 *adv)
>> +{
>> + struct device *dev = &adv->i2c_main->dev;
>> + const char * const *supply_names;
>> + int i, ret;
>
> i only takes positive values, you can make it an unsigned int.
>
> Apart from that,
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
I'll fix these. Thanks for the review.
Archit
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 52+ messages in thread
* [PATCH v2 3/4] arm64: dts: apq8016-sbc: Add some missing regulator supplies for ADV7533
2016-09-23 9:20 ` [PATCH v2 0/4] arm64: apq8016-sbc: Enable regulators for ADV7533 Archit Taneja
2016-09-23 9:20 ` [PATCH v2 1/4] dt-bindings: drm/bridge: adv7511: Add regulator bindings Archit Taneja
2016-09-23 9:20 ` [PATCH v2 2/4] drm/bridge: adv7511: Initialize regulators Archit Taneja
@ 2016-09-23 9:20 ` Archit Taneja
2016-09-23 9:20 ` [PATCH v2 4/4] arm64: dts: apq8016-sbc: Set up LDO2, LDO6 and LDO17 regulator voltage ranges Archit Taneja
2016-11-29 6:07 ` [PATCH v3 0/2] drm/bridge: adv7511: Enable regulators Archit Taneja
4 siblings, 0 replies; 52+ messages in thread
From: Archit Taneja @ 2016-09-23 9:20 UTC (permalink / raw)
To: andy.gross, laurent.pinchart; +Cc: linux-arm-msm, robh, Archit Taneja
The DT bindings were updated for ADV7533 to make sure different
1.8V pins(AVDD, DVDD, PVDD, A2VDD) on the chip have different regulator
supply entries. This was done to make sure things don't break for platforms
which source these pins from different 1.8V supplies.
Update the DT node according to new bindings. There isn't any change in
behavior between the current and new DTs since all these supplies are
fed by the same LDO.
Signed-off-by: Archit Taneja <architt@codeaurora.org>
---
arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi b/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi
index bb062b5..87c5f08 100644
--- a/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi
+++ b/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi
@@ -78,6 +78,9 @@
pd-gpios = <&msmgpio 32 0>;
avdd-supply = <&pm8916_l6>;
+ dvdd-supply = <&pm8916_l6>;
+ pvdd-supply = <&pm8916_l6>;
+ a2vdd-supply = <&pm8916_l6>;
v1p2-supply = <&pm8916_l6>;
v3p3-supply = <&pm8916_l17>;
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
^ permalink raw reply related [flat|nested] 52+ messages in thread
* [PATCH v2 4/4] arm64: dts: apq8016-sbc: Set up LDO2, LDO6 and LDO17 regulator voltage ranges
2016-09-23 9:20 ` [PATCH v2 0/4] arm64: apq8016-sbc: Enable regulators for ADV7533 Archit Taneja
` (2 preceding siblings ...)
2016-09-23 9:20 ` [PATCH v2 3/4] arm64: dts: apq8016-sbc: Add some missing regulator supplies for ADV7533 Archit Taneja
@ 2016-09-23 9:20 ` Archit Taneja
2016-11-29 6:07 ` [PATCH v3 0/2] drm/bridge: adv7511: Enable regulators Archit Taneja
4 siblings, 0 replies; 52+ messages in thread
From: Archit Taneja @ 2016-09-23 9:20 UTC (permalink / raw)
To: andy.gross, laurent.pinchart; +Cc: linux-arm-msm, robh, Archit Taneja
On the APQ8016 SBC, the LDO2 PM8916 regulator feeds 1.2V to the following:
- VDDA_1P2_MIPI_DSI and VDDA_MIPI_CSI pins on APQ8016.
- VCCCAD pins on the LPDDR3 chip.
- VDDPX_1 pins on APQ8016.
The LDO6 regulator feeds 1.8V to:
- VDAA_MIPI_DSI0_PLL pin on APQ8016.
- QFPROM_BLOW_VDD pin on PM8916.
- The AVDD, A2VDD and DVDD pins on ADV7533 bridge.
The LDO17 regulator feeds 3.3V to:
- The V3P3 pin on ADV7533 bridge.
Currently, the regulator min/max voltages for all the LDOs are set to the
range of what the PMIC supports. Set the ranges for L2, L6 and L17 to what
we need, i.e. 1.2V, 1.8V and 3.3V respectively.
Signed-off-by: Archit Taneja <architt@codeaurora.org>
---
arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi b/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi
index 87c5f08..05ba194 100644
--- a/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi
+++ b/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi
@@ -311,8 +311,8 @@
};
l2 {
- regulator-min-microvolt = <375000>;
- regulator-max-microvolt = <1525000>;
+ regulator-min-microvolt = <1200000>;
+ regulator-max-microvolt = <1200000>;
};
l3 {
@@ -331,8 +331,8 @@
};
l6 {
- regulator-min-microvolt = <1750000>;
- regulator-max-microvolt = <3337000>;
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <1800000>;
};
l7 {
@@ -391,8 +391,8 @@
};
l17 {
- regulator-min-microvolt = <1750000>;
- regulator-max-microvolt = <3337000>;
+ regulator-min-microvolt = <3300000>;
+ regulator-max-microvolt = <3300000>;
};
l18 {
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
^ permalink raw reply related [flat|nested] 52+ messages in thread
* [PATCH v3 0/2] drm/bridge: adv7511: Enable regulators
2016-09-23 9:20 ` [PATCH v2 0/4] arm64: apq8016-sbc: Enable regulators for ADV7533 Archit Taneja
` (3 preceding siblings ...)
2016-09-23 9:20 ` [PATCH v2 4/4] arm64: dts: apq8016-sbc: Set up LDO2, LDO6 and LDO17 regulator voltage ranges Archit Taneja
@ 2016-11-29 6:07 ` Archit Taneja
[not found] ` <1480399662-8858-1-git-send-email-architt-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
` (2 more replies)
4 siblings, 3 replies; 52+ messages in thread
From: Archit Taneja @ 2016-11-29 6:07 UTC (permalink / raw)
To: laurent.pinchart; +Cc: linux-arm-msm, robh, dri-devel, Archit Taneja
The patch below added regulator supplies to the ADV533 HDMI bridge
on the DB410c, but the corresponding DT binding doc wasn't updated
to list them:
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=28546b09551190c727c94d1c5c96ca609065beb2
The first patch adds the regulator supply props to the binding
docs for both ADV7511 and ADV7533. The second patch updates the drm
bridge driver to enable these regulators.
Changes in v3:
- Revert back to having a common supply for AVDD, DVDD, PVDD pins.
- Dropped the DB410c DT patches.
Changes in v2:
- Provide the regulator supply for ADV7511 too in the binding docs.
- Update the driver to manage regulators for both ADV7511 and ADV7533.
- Have separate supply entries for AVDD, DVDD, PVDD, A2VDD pins.
- Use regulator_bulk_* API to configure regulators.
Archit Taneja (2):
dt-bindings: drm/bridge: adv7511: Add regulator bindings
drm/bridge: adv7511: Initialize regulators
.../bindings/display/bridge/adi,adv7511.txt | 9 +++
drivers/gpu/drm/bridge/adv7511/adv7511.h | 4 ++
drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 79 +++++++++++++++++++---
3 files changed, 83 insertions(+), 9 deletions(-)
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
^ permalink raw reply [flat|nested] 52+ messages in thread
[parent not found: <1480399662-8858-1-git-send-email-architt-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>]
* [PATCH v3 1/2] dt-bindings: drm/bridge: adv7511: Add regulator bindings
[not found] ` <1480399662-8858-1-git-send-email-architt-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
@ 2016-11-29 6:07 ` Archit Taneja
[not found] ` <1480399662-8858-2-git-send-email-architt-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
0 siblings, 1 reply; 52+ messages in thread
From: Archit Taneja @ 2016-11-29 6:07 UTC (permalink / raw)
To: laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw
Cc: linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
robh-DgEjT+Ai2ygdnm+yROfE0A,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Archit Taneja,
devicetree-u79uwXL29TY76Z2rM5mHXA
Add the regulator supply properties needed by ADV7511 and ADV7533.
The regulators are specified as optional properties since there can
be boards which have a fixed supply directly routed to the pins, and
these may not be modelled as regulator supplies.
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Signed-off-by: Archit Taneja <architt-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
---
v3:
- Revert back to having a common avdd-supply property for the 1.8V
supplies
Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
index 6532a59..13d53bc 100644
--- a/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
+++ b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
@@ -56,6 +56,15 @@ Optional properties:
- adi,disable-timing-generator: Only for ADV7533. Disables the internal timing
generator. The chip will rely on the sync signals in the DSI data lanes,
rather than generate its own timings for HDMI output.
+- avdd-supply: A common 1.8V supply that powers up the AVDD, DVDD and PVDD
+ pins. On ADV7511, it also feeds to the BGVDD pin. On ADV7533, it also powers
+ up the A2VDD pin.
+- v3p3-supply: A 3.3V supply that powers up the pin called DVDD_3V on
+ ADV7511 and V3P3 on ADV7533.
+
+ADV7533 specific supplies:
+- v1p2-supply: A supply that powers up the V1P2 pin on the chip. It can be
+ either 1.2V or 1.8V.
Required nodes:
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 52+ messages in thread
* [PATCH v3 2/2] drm/bridge: adv7511: Initialize regulators
2016-11-29 6:07 ` [PATCH v3 0/2] drm/bridge: adv7511: Enable regulators Archit Taneja
[not found] ` <1480399662-8858-1-git-send-email-architt-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
@ 2016-11-29 6:07 ` Archit Taneja
2016-11-29 6:28 ` Laurent Pinchart
2016-12-05 7:53 ` [PATCH v4 0/2] drm/bridge: adv7511: Enable regulators Archit Taneja
2 siblings, 1 reply; 52+ messages in thread
From: Archit Taneja @ 2016-11-29 6:07 UTC (permalink / raw)
To: laurent.pinchart; +Cc: linux-arm-msm, robh, dri-devel, Archit Taneja
Maintain a table of regulator names expect by ADV7511 and ADV7533.
Use regulator_bulk_* api to configure these.
Initialize and enable the regulators during probe itself. Controlling
these dynamically is left for later.
Signed-off-by: Archit Taneja <architt@codeaurora.org>
---
v3:
- Drop the additional 1.8V supply names
drivers/gpu/drm/bridge/adv7511/adv7511.h | 4 ++
drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 79 ++++++++++++++++++++++++----
2 files changed, 74 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511.h b/drivers/gpu/drm/bridge/adv7511/adv7511.h
index 992d76c..18ec627 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511.h
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511.h
@@ -12,6 +12,7 @@
#include <linux/hdmi.h>
#include <linux/i2c.h>
#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
#include <drm/drm_crtc_helper.h>
#include <drm/drm_mipi_dsi.h>
@@ -329,6 +330,9 @@ struct adv7511 {
struct gpio_desc *gpio_pd;
+ struct regulator_bulk_data *supplies;
+ int num_supplies;
+
/* ADV7533 DSI RX related params */
struct device_node *host_node;
struct mipi_dsi_device *dsi;
diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
index 2177440..f123fbb 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
@@ -843,6 +843,51 @@ static int adv7511_bridge_attach(struct drm_bridge *bridge)
* Probe & remove
*/
+static const char * const adv7511_supply_names[] = {
+ "avdd",
+ "v3p3",
+};
+
+static const char * const adv7533_supply_names[] = {
+ "avdd",
+ "v3p3",
+ "v1p2",
+};
+
+static int adv7511_init_regulators(struct adv7511 *adv)
+{
+ struct device *dev = &adv->i2c_main->dev;
+ const char * const *supply_names;
+ int i, ret;
+
+ if (adv->type == ADV7511) {
+ supply_names = adv7511_supply_names;
+ adv->num_supplies = ARRAY_SIZE(adv7511_supply_names);
+ } else {
+ supply_names = adv7533_supply_names;
+ adv->num_supplies = ARRAY_SIZE(adv7533_supply_names);
+ }
+
+ adv->supplies = devm_kcalloc(dev, adv->num_supplies,
+ sizeof(*adv->supplies), GFP_KERNEL);
+ if (!adv->supplies)
+ return -ENOMEM;
+
+ for (i = 0; i < adv->num_supplies; i++)
+ adv->supplies[i].supply = supply_names[i];
+
+ ret = devm_regulator_bulk_get(dev, adv->num_supplies, adv->supplies);
+ if (ret)
+ return ret;
+
+ return regulator_bulk_enable(adv->num_supplies, adv->supplies);
+}
+
+static void adv7511_uninit_regulators(struct adv7511 *adv)
+{
+ regulator_bulk_disable(adv->num_supplies, adv->supplies);
+}
+
static int adv7511_parse_dt(struct device_node *np,
struct adv7511_link_config *config)
{
@@ -943,6 +988,7 @@ static int adv7511_probe(struct i2c_client *i2c, const struct i2c_device_id *id)
if (!adv7511)
return -ENOMEM;
+ adv7511->i2c_main = i2c;
adv7511->powered = false;
adv7511->status = connector_status_disconnected;
@@ -960,13 +1006,21 @@ static int adv7511_probe(struct i2c_client *i2c, const struct i2c_device_id *id)
if (ret)
return ret;
+ ret = adv7511_init_regulators(adv7511);
+ if (ret) {
+ dev_err(dev, "failed to init regulators\n");
+ return ret;
+ }
+
/*
* The power down GPIO is optional. If present, toggle it from active to
* inactive to wake up the encoder.
*/
adv7511->gpio_pd = devm_gpiod_get_optional(dev, "pd", GPIOD_OUT_HIGH);
- if (IS_ERR(adv7511->gpio_pd))
- return PTR_ERR(adv7511->gpio_pd);
+ if (IS_ERR(adv7511->gpio_pd)) {
+ ret = PTR_ERR(adv7511->gpio_pd);
+ goto uninit_regulators;
+ }
if (adv7511->gpio_pd) {
mdelay(5);
@@ -974,12 +1028,14 @@ static int adv7511_probe(struct i2c_client *i2c, const struct i2c_device_id *id)
}
adv7511->regmap = devm_regmap_init_i2c(i2c, &adv7511_regmap_config);
- if (IS_ERR(adv7511->regmap))
- return PTR_ERR(adv7511->regmap);
+ if (IS_ERR(adv7511->regmap)) {
+ ret = PTR_ERR(adv7511->regmap);
+ goto uninit_regulators;
+ }
ret = regmap_read(adv7511->regmap, ADV7511_REG_CHIP_REVISION, &val);
if (ret)
- return ret;
+ goto uninit_regulators;
dev_dbg(dev, "Rev. %d\n", val);
if (adv7511->type == ADV7511)
@@ -989,7 +1045,7 @@ static int adv7511_probe(struct i2c_client *i2c, const struct i2c_device_id *id)
else
ret = adv7533_patch_registers(adv7511);
if (ret)
- return ret;
+ goto uninit_regulators;
regmap_write(adv7511->regmap, ADV7511_REG_EDID_I2C_ADDR, edid_i2c_addr);
regmap_write(adv7511->regmap, ADV7511_REG_PACKET_I2C_ADDR,
@@ -999,10 +1055,11 @@ static int adv7511_probe(struct i2c_client *i2c, const struct i2c_device_id *id)
adv7511_packet_disable(adv7511, 0xffff);
- adv7511->i2c_main = i2c;
adv7511->i2c_edid = i2c_new_dummy(i2c->adapter, edid_i2c_addr >> 1);
- if (!adv7511->i2c_edid)
- return -ENOMEM;
+ if (!adv7511->i2c_edid) {
+ ret = -ENOMEM;
+ goto uninit_regulators;
+ }
if (adv7511->type == ADV7533) {
ret = adv7533_init_cec(adv7511);
@@ -1049,6 +1106,8 @@ static int adv7511_probe(struct i2c_client *i2c, const struct i2c_device_id *id)
adv7533_uninit_cec(adv7511);
err_i2c_unregister_edid:
i2c_unregister_device(adv7511->i2c_edid);
+uninit_regulators:
+ adv7511_uninit_regulators(adv7511);
return ret;
}
@@ -1062,6 +1121,8 @@ static int adv7511_remove(struct i2c_client *i2c)
adv7533_uninit_cec(adv7511);
}
+ adv7511_uninit_regulators(adv7511);
+
drm_bridge_remove(&adv7511->bridge);
adv7511_audio_exit(adv7511);
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
^ permalink raw reply related [flat|nested] 52+ messages in thread
* Re: [PATCH v3 2/2] drm/bridge: adv7511: Initialize regulators
2016-11-29 6:07 ` [PATCH v3 2/2] drm/bridge: adv7511: Initialize regulators Archit Taneja
@ 2016-11-29 6:28 ` Laurent Pinchart
0 siblings, 0 replies; 52+ messages in thread
From: Laurent Pinchart @ 2016-11-29 6:28 UTC (permalink / raw)
To: Archit Taneja; +Cc: linux-arm-msm, robh, dri-devel
Hi Archit,
Thank you for the patch.
On Tuesday 29 Nov 2016 11:37:42 Archit Taneja wrote:
> Maintain a table of regulator names expect by ADV7511 and ADV7533.
> Use regulator_bulk_* api to configure these.
>
> Initialize and enable the regulators during probe itself. Controlling
> these dynamically is left for later.
>
> Signed-off-by: Archit Taneja <architt@codeaurora.org>
> ---
> v3:
> - Drop the additional 1.8V supply names
>
> drivers/gpu/drm/bridge/adv7511/adv7511.h | 4 ++
> drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 79 +++++++++++++++++++++----
> 2 files changed, 74 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511.h
> b/drivers/gpu/drm/bridge/adv7511/adv7511.h index 992d76c..18ec627 100644
> --- a/drivers/gpu/drm/bridge/adv7511/adv7511.h
> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511.h
> @@ -12,6 +12,7 @@
> #include <linux/hdmi.h>
> #include <linux/i2c.h>
> #include <linux/regmap.h>
> +#include <linux/regulator/consumer.h>
>
> #include <drm/drm_crtc_helper.h>
> #include <drm/drm_mipi_dsi.h>
> @@ -329,6 +330,9 @@ struct adv7511 {
>
> struct gpio_desc *gpio_pd;
>
> + struct regulator_bulk_data *supplies;
> + int num_supplies;
num_supplies is always positive, you can make it an unsigned int.
> +
> /* ADV7533 DSI RX related params */
> struct device_node *host_node;
> struct mipi_dsi_device *dsi;
> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c index 2177440..f123fbb
> 100644
> --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> @@ -843,6 +843,51 @@ static int adv7511_bridge_attach(struct drm_bridge
> *bridge) * Probe & remove
> */
>
> +static const char * const adv7511_supply_names[] = {
> + "avdd",
> + "v3p3",
> +};
> +
> +static const char * const adv7533_supply_names[] = {
> + "avdd",
> + "v3p3",
> + "v1p2",
> +};
> +
> +static int adv7511_init_regulators(struct adv7511 *adv)
> +{
> + struct device *dev = &adv->i2c_main->dev;
> + const char * const *supply_names;
> + int i, ret;
Same comment for i.
Apart from that,
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> +
> + if (adv->type == ADV7511) {
> + supply_names = adv7511_supply_names;
> + adv->num_supplies = ARRAY_SIZE(adv7511_supply_names);
> + } else {
> + supply_names = adv7533_supply_names;
> + adv->num_supplies = ARRAY_SIZE(adv7533_supply_names);
> + }
> +
> + adv->supplies = devm_kcalloc(dev, adv->num_supplies,
> + sizeof(*adv->supplies), GFP_KERNEL);
> + if (!adv->supplies)
> + return -ENOMEM;
> +
> + for (i = 0; i < adv->num_supplies; i++)
> + adv->supplies[i].supply = supply_names[i];
> +
> + ret = devm_regulator_bulk_get(dev, adv->num_supplies, adv->supplies);
> + if (ret)
> + return ret;
> +
> + return regulator_bulk_enable(adv->num_supplies, adv->supplies);
> +}
> +
> +static void adv7511_uninit_regulators(struct adv7511 *adv)
> +{
> + regulator_bulk_disable(adv->num_supplies, adv->supplies);
> +}
> +
> static int adv7511_parse_dt(struct device_node *np,
> struct adv7511_link_config *config)
> {
> @@ -943,6 +988,7 @@ static int adv7511_probe(struct i2c_client *i2c, const
> struct i2c_device_id *id) if (!adv7511)
> return -ENOMEM;
>
> + adv7511->i2c_main = i2c;
> adv7511->powered = false;
> adv7511->status = connector_status_disconnected;
>
> @@ -960,13 +1006,21 @@ static int adv7511_probe(struct i2c_client *i2c,
> const struct i2c_device_id *id) if (ret)
> return ret;
>
> + ret = adv7511_init_regulators(adv7511);
> + if (ret) {
> + dev_err(dev, "failed to init regulators\n");
> + return ret;
> + }
> +
> /*
> * The power down GPIO is optional. If present, toggle it from active
to
> * inactive to wake up the encoder.
> */
> adv7511->gpio_pd = devm_gpiod_get_optional(dev, "pd", GPIOD_OUT_HIGH);
> - if (IS_ERR(adv7511->gpio_pd))
> - return PTR_ERR(adv7511->gpio_pd);
> + if (IS_ERR(adv7511->gpio_pd)) {
> + ret = PTR_ERR(adv7511->gpio_pd);
> + goto uninit_regulators;
> + }
>
> if (adv7511->gpio_pd) {
> mdelay(5);
> @@ -974,12 +1028,14 @@ static int adv7511_probe(struct i2c_client *i2c,
> const struct i2c_device_id *id) }
>
> adv7511->regmap = devm_regmap_init_i2c(i2c, &adv7511_regmap_config);
> - if (IS_ERR(adv7511->regmap))
> - return PTR_ERR(adv7511->regmap);
> + if (IS_ERR(adv7511->regmap)) {
> + ret = PTR_ERR(adv7511->regmap);
> + goto uninit_regulators;
> + }
>
> ret = regmap_read(adv7511->regmap, ADV7511_REG_CHIP_REVISION, &val);
> if (ret)
> - return ret;
> + goto uninit_regulators;
> dev_dbg(dev, "Rev. %d\n", val);
>
> if (adv7511->type == ADV7511)
> @@ -989,7 +1045,7 @@ static int adv7511_probe(struct i2c_client *i2c, const
> struct i2c_device_id *id) else
> ret = adv7533_patch_registers(adv7511);
> if (ret)
> - return ret;
> + goto uninit_regulators;
>
> regmap_write(adv7511->regmap, ADV7511_REG_EDID_I2C_ADDR,
edid_i2c_addr);
> regmap_write(adv7511->regmap, ADV7511_REG_PACKET_I2C_ADDR,
> @@ -999,10 +1055,11 @@ static int adv7511_probe(struct i2c_client *i2c,
> const struct i2c_device_id *id)
>
> adv7511_packet_disable(adv7511, 0xffff);
>
> - adv7511->i2c_main = i2c;
> adv7511->i2c_edid = i2c_new_dummy(i2c->adapter, edid_i2c_addr >> 1);
> - if (!adv7511->i2c_edid)
> - return -ENOMEM;
> + if (!adv7511->i2c_edid) {
> + ret = -ENOMEM;
> + goto uninit_regulators;
> + }
>
> if (adv7511->type == ADV7533) {
> ret = adv7533_init_cec(adv7511);
> @@ -1049,6 +1106,8 @@ static int adv7511_probe(struct i2c_client *i2c, const
> struct i2c_device_id *id) adv7533_uninit_cec(adv7511);
> err_i2c_unregister_edid:
> i2c_unregister_device(adv7511->i2c_edid);
> +uninit_regulators:
> + adv7511_uninit_regulators(adv7511);
>
> return ret;
> }
> @@ -1062,6 +1121,8 @@ static int adv7511_remove(struct i2c_client *i2c)
> adv7533_uninit_cec(adv7511);
> }
>
> + adv7511_uninit_regulators(adv7511);
> +
> drm_bridge_remove(&adv7511->bridge);
>
> adv7511_audio_exit(adv7511);
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 52+ messages in thread
* [PATCH v4 0/2] drm/bridge: adv7511: Enable regulators
2016-11-29 6:07 ` [PATCH v3 0/2] drm/bridge: adv7511: Enable regulators Archit Taneja
[not found] ` <1480399662-8858-1-git-send-email-architt-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
2016-11-29 6:07 ` [PATCH v3 2/2] drm/bridge: adv7511: Initialize regulators Archit Taneja
@ 2016-12-05 7:53 ` Archit Taneja
2016-12-05 7:53 ` [PATCH v4 1/2] dt-bindings: drm/bridge: adv7511: Add regulator bindings Archit Taneja
` (2 more replies)
2 siblings, 3 replies; 52+ messages in thread
From: Archit Taneja @ 2016-12-05 7:53 UTC (permalink / raw)
To: laurent.pinchart; +Cc: linux-arm-msm, robh, dri-devel, Archit Taneja
The patch below added regulator supplies to the ADV533 HDMI bridge
on the DB410c, but the corresponding DT binding doc wasn't updated
to list them:
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=28546b09551190c727c94d1c5c96ca609065beb2
The first patch adds the regulator supply props to the binding
docs for both ADV7511 and ADV7533. The second patch updates the drm
bridge driver to enable these regulators.
Changes in v4:
- Moved the regulator supply bindings from optional to mandatory.
- Used unsigned int for num_supplies and iterator used for parsing
the regulator supply properties.
Changes in v3:
- Revert back to having a common supply for AVDD, DVDD, PVDD pins.
- Dropped the DB410c DT patches.
Changes in v2:
- Provide the regulator supply for ADV7511 too in the binding docs.
- Update the driver to manage regulators for both ADV7511 and ADV7533.
- Have separate supply entries for AVDD, DVDD, PVDD, A2VDD pins.
- Use regulator_bulk_* API to configure regulators.
Archit Taneja (2):
dt-bindings: drm/bridge: adv7511: Add regulator bindings
drm/bridge: adv7511: Initialize regulators
.../bindings/display/bridge/adi,adv7511.txt | 8 +++
drivers/gpu/drm/bridge/adv7511/adv7511.h | 4 ++
drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 80 +++++++++++++++++++---
3 files changed, 83 insertions(+), 9 deletions(-)
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
^ permalink raw reply [flat|nested] 52+ messages in thread
* [PATCH v4 1/2] dt-bindings: drm/bridge: adv7511: Add regulator bindings
2016-12-05 7:53 ` [PATCH v4 0/2] drm/bridge: adv7511: Enable regulators Archit Taneja
@ 2016-12-05 7:53 ` Archit Taneja
2016-12-09 22:11 ` Rob Herring
2016-12-05 7:53 ` [PATCH v4 2/2] drm/bridge: adv7511: Initialize regulators Archit Taneja
2017-01-11 6:52 ` [PATCH v5 0/2] drm/bridge: adv7511: Enable regulators Archit Taneja
2 siblings, 1 reply; 52+ messages in thread
From: Archit Taneja @ 2016-12-05 7:53 UTC (permalink / raw)
To: laurent.pinchart; +Cc: devicetree, linux-arm-msm, dri-devel
Add the regulator supply properties needed by ADV7511 and ADV7533.
Cc: devicetree@vger.kernel.org
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Archit Taneja <architt@codeaurora.org>
---
Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
index 6532a59..00ce479 100644
--- a/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
+++ b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
@@ -38,10 +38,18 @@ The following input format properties are required except in "rgb 1x" and
- adi,input-justification: The input bit justification ("left", "evenly",
"right").
+- avdd-supply: A common 1.8V supply that powers up the AVDD, DVDD and PVDD
+ pins. On ADV7511, it also feeds to the BGVDD pin. On ADV7533, it also powers
+ up the A2VDD pin.
+- v3p3-supply: A 3.3V supply that powers up the pin called DVDD_3V on
+ ADV7511 and V3P3 on ADV7533.
+
The following properties are required for ADV7533:
- adi,dsi-lanes: Number of DSI data lanes connected to the DSI host. It should
be one of 1, 2, 3 or 4.
+- v1p2-supply: A supply that powers up the V1P2 pin on the chip. It can be
+ either 1.2V or 1.8V. This supply is specific to ADV7533.
Optional properties:
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 52+ messages in thread
* Re: [PATCH v4 1/2] dt-bindings: drm/bridge: adv7511: Add regulator bindings
2016-12-05 7:53 ` [PATCH v4 1/2] dt-bindings: drm/bridge: adv7511: Add regulator bindings Archit Taneja
@ 2016-12-09 22:11 ` Rob Herring
2016-12-18 11:17 ` Archit Taneja
0 siblings, 1 reply; 52+ messages in thread
From: Rob Herring @ 2016-12-09 22:11 UTC (permalink / raw)
To: Archit Taneja; +Cc: laurent.pinchart, linux-arm-msm, dri-devel, devicetree
On Mon, Dec 05, 2016 at 01:23:54PM +0530, Archit Taneja wrote:
> Add the regulator supply properties needed by ADV7511 and ADV7533.
>
> Cc: devicetree@vger.kernel.org
> Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Archit Taneja <architt@codeaurora.org>
> ---
> Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt | 8 ++++++++
> 1 file changed, 8 insertions(+)
Didn't I ack this already? Anyway,
Acked-by: Rob Herring <robh@kernel.org>
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH v4 1/2] dt-bindings: drm/bridge: adv7511: Add regulator bindings
2016-12-09 22:11 ` Rob Herring
@ 2016-12-18 11:17 ` Archit Taneja
0 siblings, 0 replies; 52+ messages in thread
From: Archit Taneja @ 2016-12-18 11:17 UTC (permalink / raw)
To: Rob Herring; +Cc: laurent.pinchart, linux-arm-msm, dri-devel, devicetree
On 12/10/2016 3:41 AM, Rob Herring wrote:
> On Mon, Dec 05, 2016 at 01:23:54PM +0530, Archit Taneja wrote:
>> Add the regulator supply properties needed by ADV7511 and ADV7533.
>>
>> Cc: devicetree@vger.kernel.org
>> Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>> Signed-off-by: Archit Taneja <architt@codeaurora.org>
>> ---
>> Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt | 8 ++++++++
>> 1 file changed, 8 insertions(+)
>
> Didn't I ack this already? Anyway,
You had Acked it before. This set moved the regulator bindings from
optional to mandatory, so I thought it may need you review again.
Mark Brown insisted that we have a regulator supply per pin even if
the chip's specs recommend using a common supply for them, so I'm
going to bring that back again. I'll keep your Ack for the next
revision since that's something you'd originally recommended.
Thanks,
Archit
>
> Acked-by: Rob Herring <robh@kernel.org>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply [flat|nested] 52+ messages in thread
* [PATCH v4 2/2] drm/bridge: adv7511: Initialize regulators
2016-12-05 7:53 ` [PATCH v4 0/2] drm/bridge: adv7511: Enable regulators Archit Taneja
2016-12-05 7:53 ` [PATCH v4 1/2] dt-bindings: drm/bridge: adv7511: Add regulator bindings Archit Taneja
@ 2016-12-05 7:53 ` Archit Taneja
2017-01-11 6:52 ` [PATCH v5 0/2] drm/bridge: adv7511: Enable regulators Archit Taneja
2 siblings, 0 replies; 52+ messages in thread
From: Archit Taneja @ 2016-12-05 7:53 UTC (permalink / raw)
To: laurent.pinchart; +Cc: linux-arm-msm, dri-devel
Maintain a table of regulator names expect by ADV7511 and ADV7533.
Use regulator_bulk_* api to configure these.
Initialize and enable the regulators during probe itself. Controlling
these dynamically is left for later.
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Archit Taneja <architt@codeaurora.org>
---
drivers/gpu/drm/bridge/adv7511/adv7511.h | 4 ++
drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 80 ++++++++++++++++++++++++----
2 files changed, 75 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511.h b/drivers/gpu/drm/bridge/adv7511/adv7511.h
index 992d76c..039147f 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511.h
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511.h
@@ -12,6 +12,7 @@
#include <linux/hdmi.h>
#include <linux/i2c.h>
#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
#include <drm/drm_crtc_helper.h>
#include <drm/drm_mipi_dsi.h>
@@ -329,6 +330,9 @@ struct adv7511 {
struct gpio_desc *gpio_pd;
+ struct regulator_bulk_data *supplies;
+ unsigned int num_supplies;
+
/* ADV7533 DSI RX related params */
struct device_node *host_node;
struct mipi_dsi_device *dsi;
diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
index 2177440..b5e61be 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
@@ -843,6 +843,52 @@ static int adv7511_bridge_attach(struct drm_bridge *bridge)
* Probe & remove
*/
+static const char * const adv7511_supply_names[] = {
+ "avdd",
+ "v3p3",
+};
+
+static const char * const adv7533_supply_names[] = {
+ "avdd",
+ "v3p3",
+ "v1p2",
+};
+
+static int adv7511_init_regulators(struct adv7511 *adv)
+{
+ struct device *dev = &adv->i2c_main->dev;
+ const char * const *supply_names;
+ unsigned int i;
+ int ret;
+
+ if (adv->type == ADV7511) {
+ supply_names = adv7511_supply_names;
+ adv->num_supplies = ARRAY_SIZE(adv7511_supply_names);
+ } else {
+ supply_names = adv7533_supply_names;
+ adv->num_supplies = ARRAY_SIZE(adv7533_supply_names);
+ }
+
+ adv->supplies = devm_kcalloc(dev, adv->num_supplies,
+ sizeof(*adv->supplies), GFP_KERNEL);
+ if (!adv->supplies)
+ return -ENOMEM;
+
+ for (i = 0; i < adv->num_supplies; i++)
+ adv->supplies[i].supply = supply_names[i];
+
+ ret = devm_regulator_bulk_get(dev, adv->num_supplies, adv->supplies);
+ if (ret)
+ return ret;
+
+ return regulator_bulk_enable(adv->num_supplies, adv->supplies);
+}
+
+static void adv7511_uninit_regulators(struct adv7511 *adv)
+{
+ regulator_bulk_disable(adv->num_supplies, adv->supplies);
+}
+
static int adv7511_parse_dt(struct device_node *np,
struct adv7511_link_config *config)
{
@@ -943,6 +989,7 @@ static int adv7511_probe(struct i2c_client *i2c, const struct i2c_device_id *id)
if (!adv7511)
return -ENOMEM;
+ adv7511->i2c_main = i2c;
adv7511->powered = false;
adv7511->status = connector_status_disconnected;
@@ -960,13 +1007,21 @@ static int adv7511_probe(struct i2c_client *i2c, const struct i2c_device_id *id)
if (ret)
return ret;
+ ret = adv7511_init_regulators(adv7511);
+ if (ret) {
+ dev_err(dev, "failed to init regulators\n");
+ return ret;
+ }
+
/*
* The power down GPIO is optional. If present, toggle it from active to
* inactive to wake up the encoder.
*/
adv7511->gpio_pd = devm_gpiod_get_optional(dev, "pd", GPIOD_OUT_HIGH);
- if (IS_ERR(adv7511->gpio_pd))
- return PTR_ERR(adv7511->gpio_pd);
+ if (IS_ERR(adv7511->gpio_pd)) {
+ ret = PTR_ERR(adv7511->gpio_pd);
+ goto uninit_regulators;
+ }
if (adv7511->gpio_pd) {
mdelay(5);
@@ -974,12 +1029,14 @@ static int adv7511_probe(struct i2c_client *i2c, const struct i2c_device_id *id)
}
adv7511->regmap = devm_regmap_init_i2c(i2c, &adv7511_regmap_config);
- if (IS_ERR(adv7511->regmap))
- return PTR_ERR(adv7511->regmap);
+ if (IS_ERR(adv7511->regmap)) {
+ ret = PTR_ERR(adv7511->regmap);
+ goto uninit_regulators;
+ }
ret = regmap_read(adv7511->regmap, ADV7511_REG_CHIP_REVISION, &val);
if (ret)
- return ret;
+ goto uninit_regulators;
dev_dbg(dev, "Rev. %d\n", val);
if (adv7511->type == ADV7511)
@@ -989,7 +1046,7 @@ static int adv7511_probe(struct i2c_client *i2c, const struct i2c_device_id *id)
else
ret = adv7533_patch_registers(adv7511);
if (ret)
- return ret;
+ goto uninit_regulators;
regmap_write(adv7511->regmap, ADV7511_REG_EDID_I2C_ADDR, edid_i2c_addr);
regmap_write(adv7511->regmap, ADV7511_REG_PACKET_I2C_ADDR,
@@ -999,10 +1056,11 @@ static int adv7511_probe(struct i2c_client *i2c, const struct i2c_device_id *id)
adv7511_packet_disable(adv7511, 0xffff);
- adv7511->i2c_main = i2c;
adv7511->i2c_edid = i2c_new_dummy(i2c->adapter, edid_i2c_addr >> 1);
- if (!adv7511->i2c_edid)
- return -ENOMEM;
+ if (!adv7511->i2c_edid) {
+ ret = -ENOMEM;
+ goto uninit_regulators;
+ }
if (adv7511->type == ADV7533) {
ret = adv7533_init_cec(adv7511);
@@ -1049,6 +1107,8 @@ static int adv7511_probe(struct i2c_client *i2c, const struct i2c_device_id *id)
adv7533_uninit_cec(adv7511);
err_i2c_unregister_edid:
i2c_unregister_device(adv7511->i2c_edid);
+uninit_regulators:
+ adv7511_uninit_regulators(adv7511);
return ret;
}
@@ -1062,6 +1122,8 @@ static int adv7511_remove(struct i2c_client *i2c)
adv7533_uninit_cec(adv7511);
}
+ adv7511_uninit_regulators(adv7511);
+
drm_bridge_remove(&adv7511->bridge);
adv7511_audio_exit(adv7511);
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 52+ messages in thread
* [PATCH v5 0/2] drm/bridge: adv7511: Enable regulators
2016-12-05 7:53 ` [PATCH v4 0/2] drm/bridge: adv7511: Enable regulators Archit Taneja
2016-12-05 7:53 ` [PATCH v4 1/2] dt-bindings: drm/bridge: adv7511: Add regulator bindings Archit Taneja
2016-12-05 7:53 ` [PATCH v4 2/2] drm/bridge: adv7511: Initialize regulators Archit Taneja
@ 2017-01-11 6:52 ` Archit Taneja
2017-01-11 6:52 ` [PATCH v5 1/2] dt-bindings: drm/bridge: adv7511: Add regulator bindings Archit Taneja
2017-01-11 6:52 ` [PATCH v5 2/2] drm/bridge: adv7511: Initialize regulators Archit Taneja
2 siblings, 2 replies; 52+ messages in thread
From: Archit Taneja @ 2017-01-11 6:52 UTC (permalink / raw)
To: laurent.pinchart; +Cc: linux-arm-msm, dri-devel
The patch below added regulator supplies to the ADV533 HDMI bridge
on the DB410c, but the corresponding DT binding doc wasn't updated
to list them:
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=28546b09551190c727c94d1c5c96ca609065beb2
The first patch adds the regulator supply props to the binding
docs for both ADV7511 and ADV7533. The second patch updates the drm
bridge driver to enable these regulators.
Changes in v5:
- Switch back to having individual supplies for AVDD, DVDD, PVDD
pins.
Changes in v4:
- Moved the regulator supply bindings from optional to mandatory.
- Used unsigned int for num_supplies and iterator used for parsing
the regulator supply properties.
Changes in v3:
- Revert back to having a common supply for AVDD, DVDD, PVDD pins.
- Dropped the DB410c DT patches.
Changes in v2:
- Provide the regulator supply for ADV7511 too in the binding docs.
- Update the driver to manage regulators for both ADV7511 and ADV7533.
- Have separate supply entries for AVDD, DVDD, PVDD, A2VDD pins.
- Use regulator_bulk_* API to configure regulators.
Archit Taneja (2):
dt-bindings: drm/bridge: adv7511: Add regulator bindings
drm/bridge: adv7511: Initialize regulators
.../bindings/display/bridge/adi,adv7511.txt | 12 +++
drivers/gpu/drm/bridge/adv7511/adv7511.h | 4 +
drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 86 +++++++++++++++++++---
3 files changed, 93 insertions(+), 9 deletions(-)
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 52+ messages in thread
* [PATCH v5 1/2] dt-bindings: drm/bridge: adv7511: Add regulator bindings
2017-01-11 6:52 ` [PATCH v5 0/2] drm/bridge: adv7511: Enable regulators Archit Taneja
@ 2017-01-11 6:52 ` Archit Taneja
2017-01-11 6:52 ` [PATCH v5 2/2] drm/bridge: adv7511: Initialize regulators Archit Taneja
1 sibling, 0 replies; 52+ messages in thread
From: Archit Taneja @ 2017-01-11 6:52 UTC (permalink / raw)
To: laurent.pinchart; +Cc: linux-arm-msm, dri-devel
Add the regulator supply properties needed by ADV7511 and ADV7533.
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Archit Taneja <architt@codeaurora.org>
---
v5:
- Bring back supplies for individual pins
- In v2, we had a v3p3-supply for DVDD_3V on ADV7511 and V3P3 pin
on ADV7533. We don't really need to force a common name here
(the adv7511 driver manages 2 different regulator name tables
anyway).
The supply on ADV7511 is called dvdd-3v-supply to match the
pin name.
.../devicetree/bindings/display/bridge/adi,adv7511.txt | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
index 6532a59..00ea670 100644
--- a/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
+++ b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
@@ -38,10 +38,22 @@ The following input format properties are required except in "rgb 1x" and
- adi,input-justification: The input bit justification ("left", "evenly",
"right").
+- avdd-supply: A 1.8V supply that powers up the AVDD pin on the chip.
+- dvdd-supply: A 1.8V supply that powers up the DVDD pin on the chip.
+- pvdd-supply: A 1.8V supply that powers up the PVDD pin on the chip.
+- dvdd-3v-supply: A 3.3V supply that powers up the pin called DVDD_3V
+ on the chip.
+- bgvdd-supply: A 1.8V supply that powers up the BGVDD pin. This is
+ needed only for ADV7511.
+
The following properties are required for ADV7533:
- adi,dsi-lanes: Number of DSI data lanes connected to the DSI host. It should
be one of 1, 2, 3 or 4.
+- a2vdd-supply: 1.8V supply that powers up the A2VDD pin on the chip.
+- v3p3-supply: A 3.3V supply that powers up the V3P3 pin on the chip.
+- v1p2-supply: A supply that powers up the V1P2 pin on the chip. It can be
+ either 1.2V or 1.8V.
Optional properties:
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 52+ messages in thread
* [PATCH v5 2/2] drm/bridge: adv7511: Initialize regulators
2017-01-11 6:52 ` [PATCH v5 0/2] drm/bridge: adv7511: Enable regulators Archit Taneja
2017-01-11 6:52 ` [PATCH v5 1/2] dt-bindings: drm/bridge: adv7511: Add regulator bindings Archit Taneja
@ 2017-01-11 6:52 ` Archit Taneja
1 sibling, 0 replies; 52+ messages in thread
From: Archit Taneja @ 2017-01-11 6:52 UTC (permalink / raw)
To: laurent.pinchart; +Cc: linux-arm-msm, dri-devel, Archit Taneja
Maintain a table of regulator names expected by ADV7511 and ADV7533.
Use regulator_bulk_* api to configure these.
Initialize and enable the regulators during probe itself. Controlling
these dynamically is left for later.
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Archit Taneja <architt@codeaurora.org>
---
drivers/gpu/drm/bridge/adv7511/adv7511.h | 4 ++
drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 86 +++++++++++++++++++++++++---
2 files changed, 81 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511.h b/drivers/gpu/drm/bridge/adv7511/adv7511.h
index 0396791..fe18a5d 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511.h
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511.h
@@ -12,6 +12,7 @@
#include <linux/hdmi.h>
#include <linux/i2c.h>
#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
#include <drm/drm_crtc_helper.h>
#include <drm/drm_mipi_dsi.h>
@@ -331,6 +332,9 @@ struct adv7511 {
struct gpio_desc *gpio_pd;
+ struct regulator_bulk_data *supplies;
+ unsigned int num_supplies;
+
/* ADV7533 DSI RX related params */
struct device_node *host_node;
struct mipi_dsi_device *dsi;
diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
index 24573e0..2f1aae7 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
@@ -859,6 +859,58 @@ static int adv7511_bridge_attach(struct drm_bridge *bridge)
* Probe & remove
*/
+static const char * const adv7511_supply_names[] = {
+ "avdd",
+ "dvdd",
+ "pvdd",
+ "bgvdd",
+ "dvdd-3v",
+};
+
+static const char * const adv7533_supply_names[] = {
+ "avdd",
+ "dvdd",
+ "pvdd",
+ "a2vdd",
+ "v3p3",
+ "v1p2",
+};
+
+static int adv7511_init_regulators(struct adv7511 *adv)
+{
+ struct device *dev = &adv->i2c_main->dev;
+ const char * const *supply_names;
+ unsigned int i;
+ int ret;
+
+ if (adv->type == ADV7511) {
+ supply_names = adv7511_supply_names;
+ adv->num_supplies = ARRAY_SIZE(adv7511_supply_names);
+ } else {
+ supply_names = adv7533_supply_names;
+ adv->num_supplies = ARRAY_SIZE(adv7533_supply_names);
+ }
+
+ adv->supplies = devm_kcalloc(dev, adv->num_supplies,
+ sizeof(*adv->supplies), GFP_KERNEL);
+ if (!adv->supplies)
+ return -ENOMEM;
+
+ for (i = 0; i < adv->num_supplies; i++)
+ adv->supplies[i].supply = supply_names[i];
+
+ ret = devm_regulator_bulk_get(dev, adv->num_supplies, adv->supplies);
+ if (ret)
+ return ret;
+
+ return regulator_bulk_enable(adv->num_supplies, adv->supplies);
+}
+
+static void adv7511_uninit_regulators(struct adv7511 *adv)
+{
+ regulator_bulk_disable(adv->num_supplies, adv->supplies);
+}
+
static int adv7511_parse_dt(struct device_node *np,
struct adv7511_link_config *config)
{
@@ -959,6 +1011,7 @@ static int adv7511_probe(struct i2c_client *i2c, const struct i2c_device_id *id)
if (!adv7511)
return -ENOMEM;
+ adv7511->i2c_main = i2c;
adv7511->powered = false;
adv7511->status = connector_status_disconnected;
@@ -976,13 +1029,21 @@ static int adv7511_probe(struct i2c_client *i2c, const struct i2c_device_id *id)
if (ret)
return ret;
+ ret = adv7511_init_regulators(adv7511);
+ if (ret) {
+ dev_err(dev, "failed to init regulators\n");
+ return ret;
+ }
+
/*
* The power down GPIO is optional. If present, toggle it from active to
* inactive to wake up the encoder.
*/
adv7511->gpio_pd = devm_gpiod_get_optional(dev, "pd", GPIOD_OUT_HIGH);
- if (IS_ERR(adv7511->gpio_pd))
- return PTR_ERR(adv7511->gpio_pd);
+ if (IS_ERR(adv7511->gpio_pd)) {
+ ret = PTR_ERR(adv7511->gpio_pd);
+ goto uninit_regulators;
+ }
if (adv7511->gpio_pd) {
mdelay(5);
@@ -990,12 +1051,14 @@ static int adv7511_probe(struct i2c_client *i2c, const struct i2c_device_id *id)
}
adv7511->regmap = devm_regmap_init_i2c(i2c, &adv7511_regmap_config);
- if (IS_ERR(adv7511->regmap))
- return PTR_ERR(adv7511->regmap);
+ if (IS_ERR(adv7511->regmap)) {
+ ret = PTR_ERR(adv7511->regmap);
+ goto uninit_regulators;
+ }
ret = regmap_read(adv7511->regmap, ADV7511_REG_CHIP_REVISION, &val);
if (ret)
- return ret;
+ goto uninit_regulators;
dev_dbg(dev, "Rev. %d\n", val);
if (adv7511->type == ADV7511)
@@ -1005,7 +1068,7 @@ static int adv7511_probe(struct i2c_client *i2c, const struct i2c_device_id *id)
else
ret = adv7533_patch_registers(adv7511);
if (ret)
- return ret;
+ goto uninit_regulators;
regmap_write(adv7511->regmap, ADV7511_REG_EDID_I2C_ADDR, edid_i2c_addr);
regmap_write(adv7511->regmap, ADV7511_REG_PACKET_I2C_ADDR,
@@ -1015,10 +1078,11 @@ static int adv7511_probe(struct i2c_client *i2c, const struct i2c_device_id *id)
adv7511_packet_disable(adv7511, 0xffff);
- adv7511->i2c_main = i2c;
adv7511->i2c_edid = i2c_new_dummy(i2c->adapter, edid_i2c_addr >> 1);
- if (!adv7511->i2c_edid)
- return -ENOMEM;
+ if (!adv7511->i2c_edid) {
+ ret = -ENOMEM;
+ goto uninit_regulators;
+ }
if (adv7511->type == ADV7533) {
ret = adv7533_init_cec(adv7511);
@@ -1067,6 +1131,8 @@ static int adv7511_probe(struct i2c_client *i2c, const struct i2c_device_id *id)
adv7533_uninit_cec(adv7511);
err_i2c_unregister_edid:
i2c_unregister_device(adv7511->i2c_edid);
+uninit_regulators:
+ adv7511_uninit_regulators(adv7511);
return ret;
}
@@ -1080,6 +1146,8 @@ static int adv7511_remove(struct i2c_client *i2c)
adv7533_uninit_cec(adv7511);
}
+ adv7511_uninit_regulators(adv7511);
+
drm_bridge_remove(&adv7511->bridge);
adv7511_audio_exit(adv7511);
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
^ permalink raw reply related [flat|nested] 52+ messages in thread