Re: [PATCH 06/13] arm: dts: s5pv210: aries: Add support for more devices

From: Jonathan Bakker <xc-racer2@live.ca>
To: Krzysztof Kozlowski <krzk@kernel.org>
Cc: kgene@kernel.org, robh+dt@kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-samsung-soc@vger.kernel.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	"Paweł Chmiel" <pawel.mikolaj.chmiel@gmail.com>
Subject: Re: [PATCH 06/13] arm: dts: s5pv210: aries: Add support for more devices
Date: Fri, 1 May 2020 12:43:26 -0700	[thread overview]
Message-ID: <BN6PR04MB0660515AC7EEF023734A3B40A3AB0@BN6PR04MB0660.namprd04.prod.outlook.com> (raw)
In-Reply-To: <20200501133100.GB5154@kozik-lap>

Hi Krzysztof,

On 2020-05-01 6:31 a.m., Krzysztof Kozlowski wrote:
>>
>> Well, it may not be in the bindings, but the driver definitely requires it :) See drivers/mfd/wm8994-core.c
>>
>> What's the best way of dealing with this situation?
> 
> I'll send a patch fixing the bindings. However don't you miss the
> AVDD1-supply in such case? Driver should report error.
> 

Thanks for sending the patch.

AVDD1 and DCVDD are provided internally, via drivers/regulator/wm8994.c and are enabled
by wlf,ldo1ena and wlf,ldo2ena pins.  It's a bit messy, but that's how it's done.

>>
>>>> +			CPVDD-supply = <&buck3_reg>;
>>>> +			SPKVDD1-supply = <&buck3_reg>;
>>>> +			SPKVDD2-supply = <&buck3_reg>;
>>>> +
>>>> +			wlf,gpio-cfg = <0xa101 0x8100 0x0100 0x0100 0x8100
>>>> +					0xa101 0x0100 0x8100 0x0100 0x0100
>>>> +					0x0100>;
>>>> +
>>>> +			wlf,ldo1ena = <&gpf3 4 GPIO_ACTIVE_HIGH>;
>>>> +			wlf,ldo2ena = <&gpf3 4 GPIO_ACTIVE_HIGH>;
>>>> +
>>>> +			wlf,lineout1-se;
>>>> +			wlf,lineout2-se;
>>>> +
>>>> +			assigned-clocks = <&clocks MOUT_CLKOUT>;
>>>> +			assigned-clock-rates = <0>;
>>>> +			assigned-clock-parents = <&xusbxti>;
>>>> +
>>>> +			pinctrl-names = "default";
>>>> +			pinctrl-0 = <&codec_ldo>;
>>>> +		};
>>>> +	};
>>>> +
>>>> +	i2c_accel: i2c-gpio-1 {
>>>> +		compatible = "i2c-gpio";
>>>> +		sda-gpios = <&gpj3 6 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
>>>> +		scl-gpios = <&gpj3 7 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
>>>> +		i2c-gpio,delay-us = <2>;
>>>> +		#address-cells = <1>;
>>>> +		#size-cells = <0>;
>>>> +
>>>> +		pinctrl-names = "default";
>>>> +		pinctrl-0 = <&accel_i2c_pins>;
>>>> +
>>>> +		/* bma023 accelerometer, no mainline binding */
>>>
>>> status disabled ... unless you need it for user-space I2C tools?
>>>
>>
>> Nope, but wanted to add them all in due to adding the si4709 in galaxys.  I was having
>> issues with accidentally overwriting previously named nodes (ie having i2c-gpio-0 in
>> both the common dtsi and the device specific dts) so I wanted to get the numbers
>> nailed down.
> 
> So add them with status disabled. This will document the HW without
> enabling empty I2C GPIO bus.

Sounds good, will do.

> 
> Best regards,
> Krzysztof
> 

Thanks,
Jonathan