Re: [PATCH V4 2/2] pinctrl: tegra: Add driver to configure voltage and power of io pads

[Jon Hunter]

On 25/11/16 17:49, Laxman Dewangan wrote:
> 
> On Friday 25 November 2016 10:59 PM, Jon Hunter wrote:
>> On 25/11/16 12:04, Laxman Dewangan wrote:
>>> Thanks Thierry for review.
>>>
>>> On Friday 25 November 2016 03:27 PM, Thierry Reding wrote:
>>>> * PGP Signed by an unknown key
>>>>
>>>> On Thu, Nov 24, 2016 at 02:08:54PM +0530, Laxman Dewangan wrote:
>>>>> +      NVIDIA Tegra124/210 SoC has IO pads which supports
>>>>> multi-voltage
>>>>> +      level of interfacing and deep power down mode of IO pads. The
>>>>> +      voltage of IO pads are SW configurable based on IO rail of that
>>>>> +      pads on T210. This driver provides the interface to change
>>>>> IO pad
>>>>> +      voltage and power state via pincontrol interface.
>>>> This has a lot of chip-specific text. Will all of that have to be
>>>> updated if support for new chips is added?
>>> Then saying that Tegra124 and later..
>>> Hoping, people know our chip releasing sequence as numbering are not in
>>> sequence.
>>>
>>>>> +#include <linux/regulator/consumer.h>
>>>>> +#include <soc/tegra/pmc.h>
>>>> Have you considered moving this code into the PMC driver? It seems a
>>>> little over the top to go through all of the platform device creation
>>>> and driver registration dance only to call into a public API later on.
>>> Yes, we had discussion on this and suggestion came to use the pinctrl
>>> framework.
>>> If we do in the pmc driver then we will need lots of DT processing for
>>> getting information from DT which we can directly get from the pinctrl
>>> core framework.
>>> Also client driver may need to have the control dynamically and get the
>>> IO pads from DT. So implementing all in pmc will be huge duplication
>>> over already existing framework.
>> I don't follow. We already did something similar for the Tegra DPAUX
>> driver [0].
>>
>> Cheers
>> Jon
>>
>> [0]
>> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/drivers/gpu/drm/tegra/dpaux.c?id=0751bb5c44fe1aa9494ce259d974c3d249b73a84
>>
> 
> In the above dpaux driver,  you used the pinctrl framework and its core
> functionality for the device tree interfacing and client interfacing.
> 
> The same thing I am saying here, we should not avoid the pinctrl
> framework. The client driver will use the pinctrl framework for the IO
> pad configurations, not direct PMC APIs.

Exactly, so why are you saying that by moving the code into the PMC
driver this will "need lots of DT processing for getting information
from DT"? By moving the code, we are not suggesting we don't use the
pinctrl framework, we are just suggesting we move the code. That's all.

Jon

-- 
nvpublic