diff mbox series

[v3] gpio: tegra186: Check GPIO pin permission before access.

Message ID 20221007055936.5446-1-pshete@nvidia.com
State Superseded
Headers show
Series [v3] gpio: tegra186: Check GPIO pin permission before access. | expand

Commit Message

Prathamesh Shete Oct. 7, 2022, 5:59 a.m. UTC
This change checks if we have the necessary permission to
access the GPIO. For devices that have support for virtualisation
we need to check both the TEGRA186_GPIO_VM_REG and the
TEGRA186_GPIO_SCR_REG registers. For device that do not have
virtualisation support for GPIOs we only need to check the
TEGRA186_GPIO_SCR_REG register.

Signed-off-by: Manish Bhardwaj <mbhardwaj@nvidia.com>
Signed-off-by: Prathamesh Shete <pshete@nvidia.com>
---
 drivers/gpio/gpio-tegra186.c | 78 ++++++++++++++++++++++++++++++++++++
 1 file changed, 78 insertions(+)

Comments

Jon Hunter May 23, 2023, 6:22 a.m. UTC | #1
On 17/10/2022 10:31, Linus Walleij wrote:
> On Fri, Oct 7, 2022 at 7:59 AM Prathamesh Shete <pshete@nvidia.com> wrote:
> 
>> This change checks if we have the necessary permission to
>> access the GPIO. For devices that have support for virtualisation
>> we need to check both the TEGRA186_GPIO_VM_REG and the
>> TEGRA186_GPIO_SCR_REG registers. For device that do not have
>> virtualisation support for GPIOs we only need to check the
>> TEGRA186_GPIO_SCR_REG register.
>>
>> Signed-off-by: Manish Bhardwaj <mbhardwaj@nvidia.com>
>> Signed-off-by: Prathamesh Shete <pshete@nvidia.com>
> 
> Very nice patch!
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>


I did not see this anywhere in the mainline/next. However, I also 
noticed that we don't have the correct email address for Bartosz (again).

Bartosz, let me know if you can pick this up? Thierry also ack'ed 
previously for Tegra too.

FWIW ...

Acked-by: Jon Hunter <jonathanh@nvidia.com>

Thanks
Jon
Bartosz Golaszewski May 23, 2023, 9:17 a.m. UTC | #2
On Tue, May 23, 2023 at 8:22 AM Jon Hunter <jonathanh@nvidia.com> wrote:
>
>
> On 17/10/2022 10:31, Linus Walleij wrote:
> > On Fri, Oct 7, 2022 at 7:59 AM Prathamesh Shete <pshete@nvidia.com> wrote:
> >
> >> This change checks if we have the necessary permission to
> >> access the GPIO. For devices that have support for virtualisation
> >> we need to check both the TEGRA186_GPIO_VM_REG and the
> >> TEGRA186_GPIO_SCR_REG registers. For device that do not have
> >> virtualisation support for GPIOs we only need to check the
> >> TEGRA186_GPIO_SCR_REG register.
> >>
> >> Signed-off-by: Manish Bhardwaj <mbhardwaj@nvidia.com>
> >> Signed-off-by: Prathamesh Shete <pshete@nvidia.com>
> >
> > Very nice patch!
> > Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
>
>
> I did not see this anywhere in the mainline/next. However, I also
> noticed that we don't have the correct email address for Bartosz (again).
>

I have only ever changed my address in MAINTAINERS once, so "again" is
not really the right term. And scripts/get_maintainer.pl should be
used anyway every time when submitting patches.

> Bartosz, let me know if you can pick this up? Thierry also ack'ed
> previously for Tegra too.
>
> FWIW ...
>
> Acked-by: Jon Hunter <jonathanh@nvidia.com>
>

This doesn't apply to v6.4-rc1. Prathmesh: could you rebase and resend?

Bart

> Thanks
> Jon
>
> --
> nvpublic
Jon Hunter May 23, 2023, 1:42 p.m. UTC | #3
On 23/05/2023 10:17, Bartosz Golaszewski wrote:
> On Tue, May 23, 2023 at 8:22 AM Jon Hunter <jonathanh@nvidia.com> wrote:
>>
>>
>> On 17/10/2022 10:31, Linus Walleij wrote:
>>> On Fri, Oct 7, 2022 at 7:59 AM Prathamesh Shete <pshete@nvidia.com> wrote:
>>>
>>>> This change checks if we have the necessary permission to
>>>> access the GPIO. For devices that have support for virtualisation
>>>> we need to check both the TEGRA186_GPIO_VM_REG and the
>>>> TEGRA186_GPIO_SCR_REG registers. For device that do not have
>>>> virtualisation support for GPIOs we only need to check the
>>>> TEGRA186_GPIO_SCR_REG register.
>>>>
>>>> Signed-off-by: Manish Bhardwaj <mbhardwaj@nvidia.com>
>>>> Signed-off-by: Prathamesh Shete <pshete@nvidia.com>
>>>
>>> Very nice patch!
>>> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
>>
>>
>> I did not see this anywhere in the mainline/next. However, I also
>> noticed that we don't have the correct email address for Bartosz (again).
>>
> 
> I have only ever changed my address in MAINTAINERS once, so "again" is
> not really the right term. And scripts/get_maintainer.pl should be
> used anyway every time when submitting patches.

Sorry I meant that WE did not send to the correct email AGAIN and not 
that you updated your email address :-)

Thanks
Jon
Andy Shevchenko May 23, 2023, 4:32 p.m. UTC | #4
Tue, May 23, 2023 at 02:42:52PM +0100, Jon Hunter kirjoitti:
> On 23/05/2023 10:17, Bartosz Golaszewski wrote:
> > On Tue, May 23, 2023 at 8:22 AM Jon Hunter <jonathanh@nvidia.com> wrote:
> > > On 17/10/2022 10:31, Linus Walleij wrote:

...

> > > I did not see this anywhere in the mainline/next. However, I also
> > > noticed that we don't have the correct email address for Bartosz (again).
> > 
> > I have only ever changed my address in MAINTAINERS once, so "again" is
> > not really the right term. And scripts/get_maintainer.pl should be
> > used anyway every time when submitting patches.
> 
> Sorry I meant that WE did not send to the correct email AGAIN and not that
> you updated your email address :-)

FWIW, you may look into my "smart" script [1] which automatically fills the Cc
and To WRT MAINTAINERS records.

[1]: https://lore.kernel.org/linux-gpio/Yz62XmiH8YG3Dtsf@orome/T/#t
Andy Shevchenko May 23, 2023, 4:43 p.m. UTC | #5
Tue, May 23, 2023 at 07:32:48PM +0300, andy.shevchenko@gmail.com kirjoitti:
> Tue, May 23, 2023 at 02:42:52PM +0100, Jon Hunter kirjoitti:
> > On 23/05/2023 10:17, Bartosz Golaszewski wrote:
> > > On Tue, May 23, 2023 at 8:22 AM Jon Hunter <jonathanh@nvidia.com> wrote:
> > > > On 17/10/2022 10:31, Linus Walleij wrote:

...

> > > > I did not see this anywhere in the mainline/next. However, I also
> > > > noticed that we don't have the correct email address for Bartosz (again).
> > > 
> > > I have only ever changed my address in MAINTAINERS once, so "again" is
> > > not really the right term. And scripts/get_maintainer.pl should be
> > > used anyway every time when submitting patches.
> > 
> > Sorry I meant that WE did not send to the correct email AGAIN and not that
> > you updated your email address :-)
> 
> FWIW, you may look into my "smart" script [1] which automatically fills the Cc
> and To WRT MAINTAINERS records.
> 
> [1]: https://lore.kernel.org/linux-gpio/Yz62XmiH8YG3Dtsf@orome/T/#t

Oops, wrong link, here we are:

https://github.com/andy-shev/home-bin-tools/blob/master/ge2maintainer.sh
diff mbox series

Patch

diff --git a/drivers/gpio/gpio-tegra186.c b/drivers/gpio/gpio-tegra186.c
index 54d9fa7da9c1..873b476ae8f1 100644
--- a/drivers/gpio/gpio-tegra186.c
+++ b/drivers/gpio/gpio-tegra186.c
@@ -26,6 +26,22 @@ 
 
 #define TEGRA186_GPIO_INT_ROUTE_MAPPING(p, x) (0x14 + (p) * 0x20 + (x) * 4)
 
+#define  TEGRA186_GPIO_VM			0x00
+#define  TEGRA186_GPIO_VM_RW_MASK		0x03
+#define  TEGRA186_GPIO_SCR			0x04
+#define  TEGRA186_GPIO_SCR_PIN_SIZE		0x08
+#define  TEGRA186_GPIO_SCR_PORT_SIZE		0x40
+#define  TEGRA186_GPIO_SCR_SEC_WEN		BIT(28)
+#define  TEGRA186_GPIO_SCR_SEC_REN		BIT(27)
+#define  TEGRA186_GPIO_SCR_SEC_G1W		BIT(9)
+#define  TEGRA186_GPIO_SCR_SEC_G1R		BIT(1)
+#define  TEGRA186_GPIO_FULL_ACCESS		(TEGRA186_GPIO_SCR_SEC_WEN | \
+						 TEGRA186_GPIO_SCR_SEC_REN | \
+						 TEGRA186_GPIO_SCR_SEC_G1R | \
+						 TEGRA186_GPIO_SCR_SEC_G1W)
+#define  TEGRA186_GPIO_SCR_SEC_ENABLE		(TEGRA186_GPIO_SCR_SEC_WEN | \
+						 TEGRA186_GPIO_SCR_SEC_REN)
+
 /* control registers */
 #define TEGRA186_GPIO_ENABLE_CONFIG 0x00
 #define  TEGRA186_GPIO_ENABLE_CONFIG_ENABLE BIT(0)
@@ -80,6 +96,7 @@  struct tegra_gpio_soc {
 	unsigned int num_pin_ranges;
 	const char *pinmux;
 	bool has_gte;
+	bool has_vm_support;
 };
 
 struct tegra_gpio {
@@ -129,6 +146,58 @@  static void __iomem *tegra186_gpio_get_base(struct tegra_gpio *gpio,
 	return gpio->base + offset + pin * 0x20;
 }
 
+static void __iomem *tegra186_gpio_get_secure_base(struct tegra_gpio *gpio,
+					    unsigned int pin)
+{
+	const struct tegra_gpio_port *port;
+	unsigned int offset;
+
+	port = tegra186_gpio_get_port(gpio, &pin);
+	if (!port)
+		return NULL;
+
+	offset = port->bank * 0x1000 + port->port * TEGRA186_GPIO_SCR_PORT_SIZE;
+
+	return gpio->secure + offset + pin * TEGRA186_GPIO_SCR_PIN_SIZE;
+}
+
+static inline bool tegra186_gpio_is_accessible(struct tegra_gpio *gpio, unsigned int pin)
+{
+	void __iomem *secure;
+	u32 value;
+
+	secure = tegra186_gpio_get_secure_base(gpio, pin);
+
+	if (gpio->soc->has_vm_support) {
+		value = readl(secure + TEGRA186_GPIO_VM);
+		if ((value & TEGRA186_GPIO_VM_RW_MASK) != TEGRA186_GPIO_VM_RW_MASK)
+			return false;
+	}
+
+	value = __raw_readl(secure + TEGRA186_GPIO_SCR);
+
+	if ((value & TEGRA186_GPIO_SCR_SEC_ENABLE) == 0)
+		return true;
+
+	if ((value & TEGRA186_GPIO_FULL_ACCESS) == TEGRA186_GPIO_FULL_ACCESS)
+		return true;
+
+	return false;
+}
+
+static int tegra186_init_valid_mask(struct gpio_chip *chip,
+		unsigned long *valid_mask, unsigned int ngpios)
+{
+	struct tegra_gpio *gpio = gpiochip_get_data(chip);
+	unsigned int j;
+
+	for (j = 0; j < ngpios; j++) {
+		if (!tegra186_gpio_is_accessible(gpio, j))
+			clear_bit(j, valid_mask);
+	}
+	return 0;
+}
+
 static int tegra186_gpio_get_direction(struct gpio_chip *chip,
 				       unsigned int offset)
 {
@@ -818,6 +887,7 @@  static int tegra186_gpio_probe(struct platform_device *pdev)
 	gpio->gpio.set = tegra186_gpio_set;
 	gpio->gpio.set_config = tegra186_gpio_set_config;
 	gpio->gpio.add_pin_ranges = tegra186_gpio_add_pin_ranges;
+	gpio->gpio.init_valid_mask = tegra186_init_valid_mask;
 	if (gpio->soc->has_gte) {
 		gpio->gpio.en_hw_timestamp = tegra186_gpio_en_hw_ts;
 		gpio->gpio.dis_hw_timestamp = tegra186_gpio_dis_hw_ts;
@@ -960,6 +1030,7 @@  static const struct tegra_gpio_soc tegra186_main_soc = {
 	.name = "tegra186-gpio",
 	.instance = 0,
 	.num_irqs_per_bank = 1,
+	.has_vm_support = false,
 };
 
 #define TEGRA186_AON_GPIO_PORT(_name, _bank, _port, _pins)	\
@@ -987,6 +1058,7 @@  static const struct tegra_gpio_soc tegra186_aon_soc = {
 	.name = "tegra186-gpio-aon",
 	.instance = 1,
 	.num_irqs_per_bank = 1,
+	.has_vm_support = false,
 };
 
 #define TEGRA194_MAIN_GPIO_PORT(_name, _bank, _port, _pins)	\
@@ -1042,6 +1114,7 @@  static const struct tegra_gpio_soc tegra194_main_soc = {
 	.num_pin_ranges = ARRAY_SIZE(tegra194_main_pin_ranges),
 	.pin_ranges = tegra194_main_pin_ranges,
 	.pinmux = "nvidia,tegra194-pinmux",
+	.has_vm_support = true,
 };
 
 #define TEGRA194_AON_GPIO_PORT(_name, _bank, _port, _pins)	\
@@ -1067,6 +1140,7 @@  static const struct tegra_gpio_soc tegra194_aon_soc = {
 	.instance = 1,
 	.num_irqs_per_bank = 8,
 	.has_gte = true,
+	.has_vm_support = false,
 };
 
 #define TEGRA234_MAIN_GPIO_PORT(_name, _bank, _port, _pins)	\
@@ -1111,6 +1185,7 @@  static const struct tegra_gpio_soc tegra234_main_soc = {
 	.name = "tegra234-gpio",
 	.instance = 0,
 	.num_irqs_per_bank = 8,
+	.has_vm_support = true,
 };
 
 #define TEGRA234_AON_GPIO_PORT(_name, _bank, _port, _pins)	\
@@ -1136,6 +1211,7 @@  static const struct tegra_gpio_soc tegra234_aon_soc = {
 	.name = "tegra234-gpio-aon",
 	.instance = 1,
 	.num_irqs_per_bank = 8,
+	.has_vm_support = false,
 };
 
 #define TEGRA241_MAIN_GPIO_PORT(_name, _bank, _port, _pins)	\
@@ -1166,6 +1242,7 @@  static const struct tegra_gpio_soc tegra241_main_soc = {
 	.name = "tegra241-gpio",
 	.instance = 0,
 	.num_irqs_per_bank = 8,
+	.has_vm_support = false,
 };
 
 #define TEGRA241_AON_GPIO_PORT(_name, _bank, _port, _pins)	\
@@ -1187,6 +1264,7 @@  static const struct tegra_gpio_soc tegra241_aon_soc = {
 	.name = "tegra241-gpio-aon",
 	.instance = 1,
 	.num_irqs_per_bank = 8,
+	.has_vm_support = false,
 };
 
 static const struct of_device_id tegra186_gpio_of_match[] = {