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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 28 Nov 2016 09:31:49 +0000
From:   Jon Hunter <jonathanh@...dia.com>
To:     Laxman Dewangan <ldewangan@...dia.com>,
        Thierry Reding <thierry.reding@...il.com>
CC:     <linus.walleij@...aro.org>, <robh+dt@...nel.org>,
        <mark.rutland@....com>, <swarren@...dotorg.org>,
        <gnurou@...il.com>, <joe@...ches.com>,
        <yamada.masahiro@...ionext.com>, <linux-gpio@...r.kernel.org>,
        <devicetree@...r.kernel.org>, <linux-tegra@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH V4 2/2] pinctrl: tegra: Add driver to configure voltage
 and power of io pads


On 25/11/16 17:45, Laxman Dewangan wrote:
> 
> On Friday 25 November 2016 10:56 PM, Jon Hunter wrote:
>> On 25/11/16 09:57, Thierry Reding wrote:
>>> * PGP Signed by an unknown key
>>>
>>> On Thu, Nov 24, 2016 at 02:08:54PM +0530, Laxman Dewangan wrote:
>> ...
>>
>>>> diff --git a/drivers/pinctrl/tegra/pinctrl-tegra-io-pad.c
>>>> b/drivers/pinctrl/tegra/pinctrl-tegra-io-pad.c
>>>> new file mode 100644
>>>> index 0000000..aab02d0
>>>> --- /dev/null
>>>> +++ b/drivers/pinctrl/tegra/pinctrl-tegra-io-pad.c
>>>> @@ -0,0 +1,530 @@
>>>> +/*
>>>> + * pinctrl-tegra-io-pad: IO PAD driver for configuration of IO rail
>>>> and deep
>>>> + *             Power Down mode via pinctrl framework.
>>>> + *
>>>> + * Copyright (C) 2016 NVIDIA CORPORATION. All rights reserved.
>>>> + *
>>>> + * Author: Laxman Dewangan <ldewangan@...dia.com>
>>>> + *
>>>> + * This program is free software; you can redistribute it and/or
>>>> modify
>>>> + * it under the terms of the GNU General Public License version 2 as
>>>> + * published by the Free Software Foundation.
>>>> + */
>>>> +
>>>> +#include <linux/delay.h>
>>>> +#include <linux/module.h>
>>>> +#include <linux/of.h>
>>>> +#include <linux/pinctrl/pinctrl.h>
>>>> +#include <linux/pinctrl/pinconf-generic.h>
>>>> +#include <linux/pinctrl/pinconf.h>
>>>> +#include <linux/pinctrl/pinmux.h>
>>>> +#include <linux/platform_device.h>
>>>> +#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.
>> I would prefer moving this under driver/soc/tegra as well (even if it is
>> not in the same source file) so we don't need all this public APIs.
>>
> 
> Do we really gain anything here by moving driver to drivers/soc/tegra?

Only avoid adding these public APIs. By using the pinctrl framework, it
would be nice to avoid having to still have public APIs that someone
could use directly.

> The folder drivers/pinctrl/tegra is dedicated folder for Tegra specific.
> We should keep the related driver in given subsystem until this is
> really hard to do it.
> Even if we organise and move this driver to the driver/soc/tegra, we
> will need only 3-4 APIs from the public pmc header to the private header.
> I think calling the tegra specific headers/APIs from tegra specific
> driver should be fine here.

Its OK, but I still prefer not having them at all.

Jon

-- 
nvpublic