Re: [PATCH] pinctrl-baytrail: add function mux checking in gpio pin request

Re: [PATCH] pinctrl-baytrail: add function mux checking in gpio pin request

[Darren Hart]

On 3/6/14, 5:59, "Chew Chiau Ee" <chiau.ee.chew@intel.com> wrote:

>From: Chew, Kean Ho <kean.ho.chew@intel.com>
>
>The requested gpio pin must has the func_pin_mux field set
>to GPIO function by BIOS/FW in advanced. Else, the gpio pin
>request would fail. This is to ensure that we do not expose
>any gpio pins which shall be used for alternate functions,
>for eg: wakeup pin, I/O interfaces for LPSS, etc.
>
>Signed-off-by: Chew, Kean Ho <kean.ho.chew@intel.com>
>Signed-off-by: Chew, Chiau Ee <chiau.ee.chew@intel.com>

Thanks for sending this out Chiau Ee,

Reviewed-by: Darren Hart <dvhart@linux.intel.com>

-- 
Darren Hart
Yocto Project - Linux Kernel
Intel Open Source Technology Center

[PATCH] pinctrl-baytrail: add function mux checking in gpio pin request

[Chew Chiau Ee]

From: Chew, Kean Ho <kean.ho.chew@intel.com>

The requested gpio pin must has the func_pin_mux field set
to GPIO function by BIOS/FW in advanced. Else, the gpio pin
request would fail. This is to ensure that we do not expose
any gpio pins which shall be used for alternate functions,
for eg: wakeup pin, I/O interfaces for LPSS, etc.

Signed-off-by: Chew, Kean Ho <kean.ho.chew@intel.com>
Signed-off-by: Chew, Chiau Ee <chiau.ee.chew@intel.com>
---
 drivers/pinctrl/pinctrl-baytrail.c |   42 +++++++++++++++++++++++++++++++++--
 1 files changed, 39 insertions(+), 3 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-baytrail.c b/drivers/pinctrl/pinctrl-baytrail.c
index 665b96b..bf2b3f6 100644
--- a/drivers/pinctrl/pinctrl-baytrail.c
+++ b/drivers/pinctrl/pinctrl-baytrail.c
@@ -60,6 +60,10 @@
 #define BYT_NGPIO_NCORE		28
 #define BYT_NGPIO_SUS		44
 
+#define BYT_SCORE_ACPI_UID	"1"
+#define BYT_NCORE_ACPI_UID	"2"
+#define BYT_SUS_ACPI_UID	"3"
+
 /*
  * Baytrail gpio controller consist of three separate sub-controllers called
  * SCORE, NCORE and SUS. The sub-controllers are identified by their acpi UID.
@@ -102,17 +106,17 @@ static unsigned const sus_pins[BYT_NGPIO_SUS] = {
 
 static struct pinctrl_gpio_range byt_ranges[] = {
 	{
-		.name = "1", /* match with acpi _UID in probe */
+		.name = BYT_SCORE_ACPI_UID, /* match with acpi _UID in probe */
 		.npins = BYT_NGPIO_SCORE,
 		.pins = score_pins,
 	},
 	{
-		.name = "2",
+		.name = BYT_NCORE_ACPI_UID,
 		.npins = BYT_NGPIO_NCORE,
 		.pins = ncore_pins,
 	},
 	{
-		.name = "3",
+		.name = BYT_SUS_ACPI_UID,
 		.npins = BYT_NGPIO_SUS,
 		.pins = sus_pins,
 	},
@@ -145,9 +149,41 @@ static void __iomem *byt_gpio_reg(struct gpio_chip *chip, unsigned offset,
 	return vg->reg_base + reg_offset + reg;
 }
 
+static bool is_special_pin(struct byt_gpio *vg, unsigned offset)
+{
+	/* SCORE pin 92-93 */
+	if (!strcmp(vg->range->name, BYT_SCORE_ACPI_UID) &&
+		offset >= 92 && offset <= 93)
+		return true;
+
+	/* SUS pin 11-21 */
+	if (!strcmp(vg->range->name, BYT_SUS_ACPI_UID) &&
+		offset >= 11 && offset <= 21)
+		return true;
+
+	return false;
+}
+
 static int byt_gpio_request(struct gpio_chip *chip, unsigned offset)
 {
 	struct byt_gpio *vg = to_byt_gpio(chip);
+	void __iomem *reg = byt_gpio_reg(chip, offset, BYT_CONF0_REG);
+	u32 value;
+	bool special;
+
+	/*
+	 * In most cases, func pin mux 000 means GPIO function.
+	 * But, some pins may have func pin mux 001 represents
+	 * GPIO function. Only allow user to export pin with
+	 * func pin mux preset as GPIO function by BIOS/FW.
+	 */
+	value = readl(reg) & BYT_PIN_MUX;
+	special = is_special_pin(vg, offset);
+	if ((special && value != 1) || (!special && value)) {
+		dev_err(&vg->pdev->dev,
+			"pin %u cannot be used as GPIO.\n", offset);
+		return -EINVAL;
+	}
 
 	pm_runtime_get(&vg->pdev->dev);
 
-- 
1.7.4.4

Re: [PATCH] pinctrl-baytrail: add function mux checking in gpio pin request

[Linus Walleij]

On Thu, Mar 6, 2014 at 2:59 PM, Chew Chiau Ee <chiau.ee.chew@intel.com> wrote:

> From: Chew, Kean Ho <kean.ho.chew@intel.com>
>
> The requested gpio pin must has the func_pin_mux field set
> to GPIO function by BIOS/FW in advanced. Else, the gpio pin
> request would fail. This is to ensure that we do not expose
> any gpio pins which shall be used for alternate functions,
> for eg: wakeup pin, I/O interfaces for LPSS, etc.
>
> Signed-off-by: Chew, Kean Ho <kean.ho.chew@intel.com>
> Signed-off-by: Chew, Chiau Ee <chiau.ee.chew@intel.com>

Patch applied with Darren's ACK.

This confirms my suspicion that you will not be able to
hide the pin control interface side of this hardware forever. ;-)

Mika/Mathias: any comments?

Yours,
Linus Walleij

Re: [PATCH] pinctrl-baytrail: add function mux checking in gpio pin request

[Mika Westerberg]

On Tue, Mar 11, 2014 at 11:27:13AM +0100, Linus Walleij wrote:
> On Thu, Mar 6, 2014 at 2:59 PM, Chew Chiau Ee <chiau.ee.chew@intel.com> wrote:
> 
> > From: Chew, Kean Ho <kean.ho.chew@intel.com>
> >
> > The requested gpio pin must has the func_pin_mux field set
> > to GPIO function by BIOS/FW in advanced. Else, the gpio pin
> > request would fail. This is to ensure that we do not expose
> > any gpio pins which shall be used for alternate functions,
> > for eg: wakeup pin, I/O interfaces for LPSS, etc.
> >
> > Signed-off-by: Chew, Kean Ho <kean.ho.chew@intel.com>
> > Signed-off-by: Chew, Chiau Ee <chiau.ee.chew@intel.com>
> 
> Patch applied with Darren's ACK.
> 
> This confirms my suspicion that you will not be able to
> hide the pin control interface side of this hardware forever. ;-)
> 
> Mika/Mathias: any comments?

No comments, looks good to me :)

Re: [PATCH] pinctrl-baytrail: add function mux checking in gpio pin request

[Mathias Nyman]

On 03/11/2014 12:27 PM, Linus Walleij wrote:
> On Thu, Mar 6, 2014 at 2:59 PM, Chew Chiau Ee <chiau.ee.chew@intel.com> wrote:
>
>> From: Chew, Kean Ho <kean.ho.chew@intel.com>
>>
>> The requested gpio pin must has the func_pin_mux field set
>> to GPIO function by BIOS/FW in advanced. Else, the gpio pin
>> request would fail. This is to ensure that we do not expose
>> any gpio pins which shall be used for alternate functions,
>> for eg: wakeup pin, I/O interfaces for LPSS, etc.
>>
>> Signed-off-by: Chew, Kean Ho <kean.ho.chew@intel.com>
>> Signed-off-by: Chew, Chiau Ee <chiau.ee.chew@intel.com>
>
> Patch applied with Darren's ACK.
>
> This confirms my suspicion that you will not be able to
> hide the pin control interface side of this hardware forever. ;-)

Hate to admit it but so it seems :)

>
> Mika/Mathias: any comments?

Patch looks fine

Acked-by: Mathias Nyman <mathias.nyman@linux.intel.com>

-Mathias