[RFC PATCH 02/13] drm/tegra: Add helper functions for setting up DPAUX pads

[Jon Hunter]

On 17/06/16 17:11, Thierry Reding wrote:
> * PGP Signed by an unknown key
> 
> On Fri, Jun 17, 2016 at 01:03:36PM +0100, Jon Hunter wrote:
>> In preparation for adding pinctrl support for the DPAUX pads, add
>> helpers functions for configuring the pads and controlling the power
>> for the pads.
>>
>> Please note that although a simple if-statement could be used instead
>> of a case statement for configuring the pads as there are only two
>> possible modes, a case statement is used because when integrating with
>> the pinctrl framework, we need to be able to handle invalid modes that
>> could be passed.
>>
>> Signed-off-by: Jon Hunter <jonathanh at nvidia.com>
>> ---
>>  drivers/gpu/drm/tegra/dpaux.c | 75 ++++++++++++++++++++++++++-----------------
>>  1 file changed, 45 insertions(+), 30 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/tegra/dpaux.c b/drivers/gpu/drm/tegra/dpaux.c
>> index 0874a7e5b37b..aa3a037fcd3b 100644
>> --- a/drivers/gpu/drm/tegra/dpaux.c
>> +++ b/drivers/gpu/drm/tegra/dpaux.c
>> @@ -267,6 +267,45 @@ static irqreturn_t tegra_dpaux_irq(int irq, void *data)
>>  	return ret;
>>  }
>>  
>> +static void tegra_dpaux_powerdown(struct tegra_dpaux *dpaux, bool enable)
>> +{
>> +	u32 value = tegra_dpaux_readl(dpaux, DPAUX_HYBRID_SPARE);
>> +
>> +	if (enable)
>> +		value |= DPAUX_HYBRID_SPARE_PAD_POWER_DOWN;
>> +	else
>> +		value &= ~DPAUX_HYBRID_SPARE_PAD_POWER_DOWN;
>> +
>> +	tegra_dpaux_writel(dpaux, value, DPAUX_HYBRID_SPARE);
>> +}
> 
> I'd like for this to be two functions without the boolean parameter. The
> reason is that without looking at the implementation there's no way to
> understand what the meaning of true and false is. If instead you call
> this:
> 
> 	static void tegra_dpaux_pad_power_down(struct tegra_dpaux *dpaux)
> 	{
> 		...
> 	}
> 
> and
> 
> 	static void tegra_dpaux_pad_power_up(struct tegra_dpaux *dpaux)
> 	{
> 		...
> 	}
> 
> you can easily deduce from the name what's going on.

OK, fine with me. I was not sure if it was obvious or not.

>> +static int tegra_dpaux_config(struct tegra_dpaux *dpaux, int function)
> 
> Can function not be unsigned?

Yes it can and in fact should be. I had thought that it needed to be
signed to match with the pinctrl-ops but on closer inspection that is
not the case.

Jon

-- 
nvpublic