Re: [RFC PATCH 10/13] drm/tegra: Add pinctrl support for DPAUX

[Jon Hunter <jonathanh@nvidia.com>]

On 17/06/16 17:37, Thierry Reding wrote:
> * PGP Signed by an unknown key
> 
> On Fri, Jun 17, 2016 at 01:03:44PM +0100, Jon Hunter wrote:
>> The DPAUX pins are shared with an internal I2C controller. To allow
>> these pins to be muxed to the I2C controller, register a pinctrl device
>> for the DPAUX device. Make Tegra DRM support dependent on PINCTRL to
>> avoid any compilation issues.
>>
>> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
>> ---
>>  drivers/gpu/drm/tegra/Kconfig |   1 +
>>  drivers/gpu/drm/tegra/dpaux.c | 117 ++++++++++++++++++++++++++++++++++++++++--
>>  2 files changed, 115 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/tegra/Kconfig b/drivers/gpu/drm/tegra/Kconfig
>> index 63ebb154b9b5..d34937a96f94 100644
>> --- a/drivers/gpu/drm/tegra/Kconfig
>> +++ b/drivers/gpu/drm/tegra/Kconfig
>> @@ -4,6 +4,7 @@ config DRM_TEGRA
>>  	depends on COMMON_CLK
>>  	depends on DRM
>>  	depends on RESET_CONTROLLER
>> +	depends on PINCTRL
> 
> Could we instead make the code optional? I don't care much about pulling
> in the extra dependency (for Tegra we always enable PINCTRL anyway), but
> I worry that somebody may end up searching for DRM_TEGRA and not find it
> because PINCTRL happens to be disabled in they .config.

Yes we could if you don't mind the #ifdef in the source file. The
alternative here would be to select PINCTRL and like you said this
should always be the case as Tegra selects in anyway.

>> diff --git a/drivers/gpu/drm/tegra/dpaux.c b/drivers/gpu/drm/tegra/dpaux.c
> [...]
>> @@ -439,6 +537,19 @@ static int tegra_dpaux_probe(struct platform_device *pdev)
>>  	if (err < 0)
>>  		return err;
>>  
>> +	dpaux->desc.name = dev_name(&pdev->dev);
>> +	dpaux->desc.pins = tegra_dpaux_pins;
>> +	dpaux->desc.npins = ARRAY_SIZE(tegra_dpaux_pins);
>> +	dpaux->desc.pctlops = &tegra_dpaux_pinctrl_ops;
>> +	dpaux->desc.pmxops = &tegra_dpaux_pinmux_ops;
>> +	dpaux->desc.owner = THIS_MODULE;
>> +
>> +	dpaux->pinctrl = pinctrl_register(&dpaux->desc, &pdev->dev, dpaux);
>> +	if (!dpaux->pinctrl) {
>> +		dev_err(&pdev->dev, "failed to register pincontrol\n");
>> +		return -ENODEV;
>> +	}
> 
> Did you mean to use the devm_ variant here? Because I don't see a
> pinctrl_unregister() in tegra_dpaux_remove().

Yes good catch. Will fix.

Jon

-- 
nvpublic
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel