diff mbox

ARM: shmobile: stout: enable R-Car Gen2 regulator quirk

Message ID 20180213232812.31408-1-marek.vasut+renesas@gmail.com (mailing list archive)
State Superseded
Delegated to: Simon Horman
Headers show

Commit Message

Marek Vasut Feb. 13, 2018, 11:28 p.m. UTC
Regulator setup is suboptimal on H2 Stout too.

Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
Cc: Geert Uytterhoeven <geert+renesas@glider.be>
Cc: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Cc: Simon Horman <horms+renesas@verge.net.au>
Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 arch/arm/mach-shmobile/regulator-quirk-rcar-gen2.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

Comments

Wolfram Sang Feb. 14, 2018, 5:58 a.m. UTC | #1
> - * The r8a7790/lager and r8a7791/koelsch development boards have da9063 and
> - * da9210 regulators.  Both regulators have their interrupt request lines tied
> - * to the same interrupt pin (IRQ2) on the SoC.
> + * The r8a7790/lager,stout and r8a7791/koelsch development boards have da9063
> + * and da9210 regulators.  Both regulators have their interrupt request lines
> + * tied to the same interrupt pin (IRQ2) on the SoC.

I think listing the boards here doesn't scale well. Gose is already
missing. How about rephrasing the paragraph to something like "Some Gen2
development boards have..."?

>   *
>   * After cold boot or da9063-induced restart, both the da9063 and da9210 seem
>   * to assert their interrupt request lines.  Hence as soon as one driver
> @@ -118,6 +118,7 @@ static int __init rcar_gen2_regulator_quirk(void)
>  
>  	if (!of_machine_is_compatible("renesas,koelsch") &&
>  	    !of_machine_is_compatible("renesas,lager") &&
> +	    !of_machine_is_compatible("renesas,stout") &&
>  	    !of_machine_is_compatible("renesas,gose"))
>  		return -ENODEV;
>  
> -- 
> 2.15.1
>
Geert Uytterhoeven Feb. 14, 2018, 8:09 a.m. UTC | #2
On Wed, Feb 14, 2018 at 6:58 AM, Wolfram Sang <wsa@the-dreams.de> wrote:
>> - * The r8a7790/lager and r8a7791/koelsch development boards have da9063 and
>> - * da9210 regulators.  Both regulators have their interrupt request lines tied
>> - * to the same interrupt pin (IRQ2) on the SoC.
>> + * The r8a7790/lager,stout and r8a7791/koelsch development boards have da9063
>> + * and da9210 regulators.  Both regulators have their interrupt request lines
>> + * tied to the same interrupt pin (IRQ2) on the SoC.
>
> I think listing the boards here doesn't scale well. Gose is already
> missing. How about rephrasing the paragraph to something like "Some Gen2
> development boards have..."?

+1

>>   *
>>   * After cold boot or da9063-induced restart, both the da9063 and da9210 seem
>>   * to assert their interrupt request lines.  Hence as soon as one driver
>> @@ -118,6 +118,7 @@ static int __init rcar_gen2_regulator_quirk(void)
>>
>>       if (!of_machine_is_compatible("renesas,koelsch") &&
>>           !of_machine_is_compatible("renesas,lager") &&
>> +         !of_machine_is_compatible("renesas,stout") &&
>>           !of_machine_is_compatible("renesas,gose"))
>>               return -ENODEV;

Have we reached critical mass to start using array-based matching with
of_device_compatible_match()?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

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
Geert Uytterhoeven Feb. 14, 2018, 9:14 a.m. UTC | #3
Hi Marek,

On Wed, Feb 14, 2018 at 12:28 AM, Marek Vasut <marek.vasut@gmail.com> wrote:
> Regulator setup is suboptimal on H2 Stout too.

Worse, Stout has 2 DA9210 regulators, so you have to add a check for a
DA9210 at address 0x70.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

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
Marek Vasut Feb. 15, 2018, 9:36 a.m. UTC | #4
On 02/14/2018 10:14 AM, Geert Uytterhoeven wrote:
> Hi Marek,
> 
> On Wed, Feb 14, 2018 at 12:28 AM, Marek Vasut <marek.vasut@gmail.com> wrote:
>> Regulator setup is suboptimal on H2 Stout too.
> 
> Worse, Stout has 2 DA9210 regulators, so you have to add a check for a
> DA9210 at address 0x70.

Fixed
Marek Vasut Feb. 15, 2018, 9:44 a.m. UTC | #5
On 02/14/2018 09:09 AM, Geert Uytterhoeven wrote:
> On Wed, Feb 14, 2018 at 6:58 AM, Wolfram Sang <wsa@the-dreams.de> wrote:
>>> - * The r8a7790/lager and r8a7791/koelsch development boards have da9063 and
>>> - * da9210 regulators.  Both regulators have their interrupt request lines tied
>>> - * to the same interrupt pin (IRQ2) on the SoC.
>>> + * The r8a7790/lager,stout and r8a7791/koelsch development boards have da9063
>>> + * and da9210 regulators.  Both regulators have their interrupt request lines
>>> + * tied to the same interrupt pin (IRQ2) on the SoC.
>>
>> I think listing the boards here doesn't scale well. Gose is already
>> missing. How about rephrasing the paragraph to something like "Some Gen2
>> development boards have..."?
> 
> +1
> 
>>>   *
>>>   * After cold boot or da9063-induced restart, both the da9063 and da9210 seem
>>>   * to assert their interrupt request lines.  Hence as soon as one driver
>>> @@ -118,6 +118,7 @@ static int __init rcar_gen2_regulator_quirk(void)
>>>
>>>       if (!of_machine_is_compatible("renesas,koelsch") &&
>>>           !of_machine_is_compatible("renesas,lager") &&
>>> +         !of_machine_is_compatible("renesas,stout") &&
>>>           !of_machine_is_compatible("renesas,gose"))
>>>               return -ENODEV;
> 
> Have we reached critical mass to start using array-based matching with
> of_device_compatible_match()?

We're matching on machine , not device , here . I guess our device node
would be / ?
Geert Uytterhoeven Feb. 15, 2018, 10:08 a.m. UTC | #6
Hi Marek,

On Thu, Feb 15, 2018 at 10:44 AM, Marek Vasut <marek.vasut@gmail.com> wrote:
> On 02/14/2018 09:09 AM, Geert Uytterhoeven wrote:
>> On Wed, Feb 14, 2018 at 6:58 AM, Wolfram Sang <wsa@the-dreams.de> wrote:
>>>> - * The r8a7790/lager and r8a7791/koelsch development boards have da9063 and
>>>> - * da9210 regulators.  Both regulators have their interrupt request lines tied
>>>> - * to the same interrupt pin (IRQ2) on the SoC.
>>>> + * The r8a7790/lager,stout and r8a7791/koelsch development boards have da9063
>>>> + * and da9210 regulators.  Both regulators have their interrupt request lines
>>>> + * tied to the same interrupt pin (IRQ2) on the SoC.
>>>
>>> I think listing the boards here doesn't scale well. Gose is already
>>> missing. How about rephrasing the paragraph to something like "Some Gen2
>>> development boards have..."?
>>
>> +1
>>
>>>>   *
>>>>   * After cold boot or da9063-induced restart, both the da9063 and da9210 seem
>>>>   * to assert their interrupt request lines.  Hence as soon as one driver
>>>> @@ -118,6 +118,7 @@ static int __init rcar_gen2_regulator_quirk(void)
>>>>
>>>>       if (!of_machine_is_compatible("renesas,koelsch") &&
>>>>           !of_machine_is_compatible("renesas,lager") &&
>>>> +         !of_machine_is_compatible("renesas,stout") &&
>>>>           !of_machine_is_compatible("renesas,gose"))
>>>>               return -ENODEV;
>>
>> Have we reached critical mass to start using array-based matching with
>> of_device_compatible_match()?
>
> We're matching on machine , not device , here . I guess our device node
> would be / ?

Correct, cfr. the implementation of of_machine_is_compatible().

BTW, several PPC platforms use of_device_compatible_match(of_root, ...),
but I believe of_root is not guaranteed to be set up.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

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
Marek Vasut Feb. 15, 2018, 10:13 a.m. UTC | #7
On 02/15/2018 11:08 AM, Geert Uytterhoeven wrote:
> Hi Marek,
> 
> On Thu, Feb 15, 2018 at 10:44 AM, Marek Vasut <marek.vasut@gmail.com> wrote:
>> On 02/14/2018 09:09 AM, Geert Uytterhoeven wrote:
>>> On Wed, Feb 14, 2018 at 6:58 AM, Wolfram Sang <wsa@the-dreams.de> wrote:
>>>>> - * The r8a7790/lager and r8a7791/koelsch development boards have da9063 and
>>>>> - * da9210 regulators.  Both regulators have their interrupt request lines tied
>>>>> - * to the same interrupt pin (IRQ2) on the SoC.
>>>>> + * The r8a7790/lager,stout and r8a7791/koelsch development boards have da9063
>>>>> + * and da9210 regulators.  Both regulators have their interrupt request lines
>>>>> + * tied to the same interrupt pin (IRQ2) on the SoC.
>>>>
>>>> I think listing the boards here doesn't scale well. Gose is already
>>>> missing. How about rephrasing the paragraph to something like "Some Gen2
>>>> development boards have..."?
>>>
>>> +1
>>>
>>>>>   *
>>>>>   * After cold boot or da9063-induced restart, both the da9063 and da9210 seem
>>>>>   * to assert their interrupt request lines.  Hence as soon as one driver
>>>>> @@ -118,6 +118,7 @@ static int __init rcar_gen2_regulator_quirk(void)
>>>>>
>>>>>       if (!of_machine_is_compatible("renesas,koelsch") &&
>>>>>           !of_machine_is_compatible("renesas,lager") &&
>>>>> +         !of_machine_is_compatible("renesas,stout") &&
>>>>>           !of_machine_is_compatible("renesas,gose"))
>>>>>               return -ENODEV;
>>>
>>> Have we reached critical mass to start using array-based matching with
>>> of_device_compatible_match()?
>>
>> We're matching on machine , not device , here . I guess our device node
>> would be / ?
> 
> Correct, cfr. the implementation of of_machine_is_compatible().
> 
> BTW, several PPC platforms use of_device_compatible_match(of_root, ...),
> but I believe of_root is not guaranteed to be set up.

OK, subsequent patch then. Added to TODO.
Wolfram Sang Feb. 15, 2018, 10:44 a.m. UTC | #8
> > BTW, several PPC platforms use of_device_compatible_match(of_root, ...),
> > but I believe of_root is not guaranteed to be set up.
> 
> OK, subsequent patch then. Added to TODO.

Can you place it below the PCIE patches, please? ;)
Marek Vasut Feb. 15, 2018, 10:53 a.m. UTC | #9
On 02/15/2018 11:44 AM, Wolfram Sang wrote:
> 
>>> BTW, several PPC platforms use of_device_compatible_match(of_root, ...),
>>> but I believe of_root is not guaranteed to be set up.
>>
>> OK, subsequent patch then. Added to TODO.
> 
> Can you place it below the PCIE patches, please? ;)

Grumble, yeah.
diff mbox

Patch

diff --git a/arch/arm/mach-shmobile/regulator-quirk-rcar-gen2.c b/arch/arm/mach-shmobile/regulator-quirk-rcar-gen2.c
index 44438f344dc8..b749450d361f 100644
--- a/arch/arm/mach-shmobile/regulator-quirk-rcar-gen2.c
+++ b/arch/arm/mach-shmobile/regulator-quirk-rcar-gen2.c
@@ -1,9 +1,9 @@ 
 /*
  * R-Car Generation 2 da9063/da9210 regulator quirk
  *
- * The r8a7790/lager and r8a7791/koelsch development boards have da9063 and
- * da9210 regulators.  Both regulators have their interrupt request lines tied
- * to the same interrupt pin (IRQ2) on the SoC.
+ * The r8a7790/lager,stout and r8a7791/koelsch development boards have da9063
+ * and da9210 regulators.  Both regulators have their interrupt request lines
+ * tied to the same interrupt pin (IRQ2) on the SoC.
  *
  * After cold boot or da9063-induced restart, both the da9063 and da9210 seem
  * to assert their interrupt request lines.  Hence as soon as one driver
@@ -118,6 +118,7 @@  static int __init rcar_gen2_regulator_quirk(void)
 
 	if (!of_machine_is_compatible("renesas,koelsch") &&
 	    !of_machine_is_compatible("renesas,lager") &&
+	    !of_machine_is_compatible("renesas,stout") &&
 	    !of_machine_is_compatible("renesas,gose"))
 		return -ENODEV;