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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxx>
> ---
>  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.

> +static int tegra_dpaux_config(struct tegra_dpaux *dpaux, int function)

Can function not be unsigned?

Thierry

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Linux GPIO]     [Linux SPI]     [Linux Hardward Monitoring]     [LM Sensors]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux