* [U-Boot] [PATCH 1/2] gpio: tegra: remove unused type @ 2015-09-25 16:44 Stephen Warren 2015-09-25 16:44 ` [U-Boot] [PATCH 2/2] gpio: tegra: use named constants Stephen Warren 2015-10-01 23:00 ` [U-Boot] [PATCH 1/2] gpio: tegra: remove unused type Simon Glass 0 siblings, 2 replies; 7+ messages in thread From: Stephen Warren @ 2015-09-25 16:44 UTC (permalink / raw) To: u-boot From: Stephen Warren <swarren@nvidia.com> These enum values aren't used anywhere. Remove them. Signed-off-by: Stephen Warren <swarren@nvidia.com> --- This series depends on my previous "ARM: tegra: don't enable GPIOs until direction is set" drivers/gpio/tegra_gpio.c | 7 ------- 1 file changed, 7 deletions(-) diff --git a/drivers/gpio/tegra_gpio.c b/drivers/gpio/tegra_gpio.c index 2dfd02d62053..f9c06fe88b71 100644 --- a/drivers/gpio/tegra_gpio.c +++ b/drivers/gpio/tegra_gpio.c @@ -25,13 +25,6 @@ DECLARE_GLOBAL_DATA_PTR; -enum { - TEGRA_CMD_INFO, - TEGRA_CMD_PORT, - TEGRA_CMD_OUTPUT, - TEGRA_CMD_INPUT, -}; - struct tegra_gpio_platdata { struct gpio_ctlr_bank *bank; const char *port_name; /* Name of port, e.g. "B" */ -- 1.9.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [U-Boot] [PATCH 2/2] gpio: tegra: use named constants 2015-09-25 16:44 [U-Boot] [PATCH 1/2] gpio: tegra: remove unused type Stephen Warren @ 2015-09-25 16:44 ` Stephen Warren 2015-10-01 23:00 ` Simon Glass 2015-10-01 23:00 ` [U-Boot] [PATCH 1/2] gpio: tegra: remove unused type Simon Glass 1 sibling, 1 reply; 7+ messages in thread From: Stephen Warren @ 2015-09-25 16:44 UTC (permalink / raw) To: u-boot From: Stephen Warren <swarren@nvidia.com> In order to make it clear what the parameters to set_config() and set_direction() mean, and similarly for the return values from the respective get_*(), define named constants for these values. Disassembly shows no diff in the generated code, except that the order of the code in the branches of tegra_gpio_get_function() gets modified without affecting behaviour. Suggested-by: Tom Warren <twarren@nvidia.com> Signed-off-by: Stephen Warren <swarren@nvidia.com> --- drivers/gpio/tegra_gpio.c | 33 +++++++++++++++++++-------------- 1 file changed, 19 insertions(+), 14 deletions(-) diff --git a/drivers/gpio/tegra_gpio.c b/drivers/gpio/tegra_gpio.c index f9c06fe88b71..8e880e276f0a 100644 --- a/drivers/gpio/tegra_gpio.c +++ b/drivers/gpio/tegra_gpio.c @@ -1,6 +1,6 @@ /* * NVIDIA Tegra20 GPIO handling. - * (C) Copyright 2010-2012 + * (C) Copyright 2010-2012,2015 * NVIDIA Corporation <www.nvidia.com> * * SPDX-License-Identifier: GPL-2.0+ @@ -25,6 +25,11 @@ DECLARE_GLOBAL_DATA_PTR; +static const int CONFIG_SFIO = 0; +static const int CONFIG_GPIO = 1; +static const int DIRECTION_INPUT = 0; +static const int DIRECTION_OUTPUT = 1; + struct tegra_gpio_platdata { struct gpio_ctlr_bank *bank; const char *port_name; /* Name of port, e.g. "B" */ @@ -37,7 +42,7 @@ struct tegra_port_info { int base_gpio; /* Port number for this port (0, 1,.., n-1) */ }; -/* Return config of pin 'gpio' as GPIO (1) or SFPIO (0) */ +/* Return config of pin 'gpio' as GPIO (1) or SFIO (0) */ static int get_config(unsigned gpio) { struct gpio_ctlr *ctlr = (struct gpio_ctlr *)NV_PA_GPIO_BASE; @@ -46,15 +51,15 @@ static int get_config(unsigned gpio) int type; u = readl(&bank->gpio_config[GPIO_PORT(gpio)]); - type = (u >> GPIO_BIT(gpio)) & 1; + type = (u >> GPIO_BIT(gpio)) & 1; debug("get_config: port = %d, bit = %d is %s\n", GPIO_FULLPORT(gpio), GPIO_BIT(gpio), type ? "GPIO" : "SFPIO"); - return type; + return type ? CONFIG_GPIO : CONFIG_SFIO; } -/* Config pin 'gpio' as GPIO or SFPIO, based on 'type' */ +/* Config pin 'gpio' as GPIO or SFIO, based on 'type' */ static void set_config(unsigned gpio, int type) { struct gpio_ctlr *ctlr = (struct gpio_ctlr *)NV_PA_GPIO_BASE; @@ -65,7 +70,7 @@ static void set_config(unsigned gpio, int type) GPIO_FULLPORT(gpio), GPIO_BIT(gpio), type ? "GPIO" : "SFPIO"); u = readl(&bank->gpio_config[GPIO_PORT(gpio)]); - if (type) /* GPIO */ + if (type != CONFIG_SFIO) u |= 1 << GPIO_BIT(gpio); else u &= ~(1 << GPIO_BIT(gpio)); @@ -86,7 +91,7 @@ static int get_direction(unsigned gpio) debug("get_direction: port = %d, bit = %d, %s\n", GPIO_FULLPORT(gpio), GPIO_BIT(gpio), dir ? "OUT" : "IN"); - return dir; + return dir ? DIRECTION_OUTPUT : DIRECTION_INPUT; } /* Config GPIO pin 'gpio' as input or output (OE) as per 'output' */ @@ -100,7 +105,7 @@ static void set_direction(unsigned gpio, int output) GPIO_FULLPORT(gpio), GPIO_BIT(gpio), output ? "OUT" : "IN"); u = readl(&bank->gpio_dir_out[GPIO_PORT(gpio)]); - if (output) + if (output != DIRECTION_INPUT) u |= 1 << GPIO_BIT(gpio); else u &= ~(1 << GPIO_BIT(gpio)); @@ -135,7 +140,7 @@ static int tegra_gpio_direction_input(struct udevice *dev, unsigned offset) struct tegra_port_info *state = dev_get_priv(dev); /* Configure GPIO direction as input. */ - set_direction(state->base_gpio + offset, 0); + set_direction(state->base_gpio + offset, DIRECTION_INPUT); /* Enable the pin as a GPIO */ set_config(state->base_gpio + offset, 1); @@ -154,7 +159,7 @@ static int tegra_gpio_direction_output(struct udevice *dev, unsigned offset, set_level(gpio, value); /* Configure GPIO direction as output. */ - set_direction(gpio, 1); + set_direction(gpio, DIRECTION_OUTPUT); /* Enable the pin as a GPIO */ set_config(state->base_gpio + offset, 1); @@ -199,18 +204,18 @@ void gpio_config_table(const struct tegra_gpio_config *config, int len) for (i = 0; i < len; i++) { switch (config[i].init) { case TEGRA_GPIO_INIT_IN: - set_direction(config[i].gpio, 0); + set_direction(config[i].gpio, DIRECTION_INPUT); break; case TEGRA_GPIO_INIT_OUT0: set_level(config[i].gpio, 0); - set_direction(config[i].gpio, 1); + set_direction(config[i].gpio, DIRECTION_OUTPUT); break; case TEGRA_GPIO_INIT_OUT1: set_level(config[i].gpio, 1); - set_direction(config[i].gpio, 1); + set_direction(config[i].gpio, DIRECTION_OUTPUT); break; } - set_config(config[i].gpio, 1); + set_config(config[i].gpio, CONFIG_GPIO); } } -- 1.9.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [U-Boot] [PATCH 2/2] gpio: tegra: use named constants 2015-09-25 16:44 ` [U-Boot] [PATCH 2/2] gpio: tegra: use named constants Stephen Warren @ 2015-10-01 23:00 ` Simon Glass 2015-10-01 23:29 ` Stephen Warren 0 siblings, 1 reply; 7+ messages in thread From: Simon Glass @ 2015-10-01 23:00 UTC (permalink / raw) To: u-boot Hi Stephen. On Friday, 25 September 2015, Stephen Warren <swarren@wwwdotorg.org> wrote: > > From: Stephen Warren <swarren@nvidia.com> > > In order to make it clear what the parameters to set_config() and > set_direction() mean, and similarly for the return values from the > respective get_*(), define named constants for these values. > > Disassembly shows no diff in the generated code, except that the > order of the code in the branches of tegra_gpio_get_function() gets > modified without affecting behaviour. > > Suggested-by: Tom Warren <twarren@nvidia.com> > Signed-off-by: Stephen Warren <swarren@nvidia.com> > --- > drivers/gpio/tegra_gpio.c | 33 +++++++++++++++++++-------------- > 1 file changed, 19 insertions(+), 14 deletions(-) > > diff --git a/drivers/gpio/tegra_gpio.c b/drivers/gpio/tegra_gpio.c > index f9c06fe88b71..8e880e276f0a 100644 > --- a/drivers/gpio/tegra_gpio.c > +++ b/drivers/gpio/tegra_gpio.c > @@ -1,6 +1,6 @@ > /* > * NVIDIA Tegra20 GPIO handling. > - * (C) Copyright 2010-2012 > + * (C) Copyright 2010-2012,2015 > * NVIDIA Corporation <www.nvidia.com> > * > * SPDX-License-Identifier: GPL-2.0+ > @@ -25,6 +25,11 @@ > > DECLARE_GLOBAL_DATA_PTR; > > +static const int CONFIG_SFIO = 0; > +static const int CONFIG_GPIO = 1; > +static const int DIRECTION_INPUT = 0; > +static const int DIRECTION_OUTPUT = 1; > + Why not use an enum? > > struct tegra_gpio_platdata { > struct gpio_ctlr_bank *bank; > const char *port_name; /* Name of port, e.g. "B" */ > @@ -37,7 +42,7 @@ struct tegra_port_info { > int base_gpio; /* Port number for this port (0, 1,.., n-1) */ > }; > > -/* Return config of pin 'gpio' as GPIO (1) or SFPIO (0) */ > +/* Return config of pin 'gpio' as GPIO (1) or SFIO (0) */ > static int get_config(unsigned gpio) > { > struct gpio_ctlr *ctlr = (struct gpio_ctlr *)NV_PA_GPIO_BASE; > @@ -46,15 +51,15 @@ static int get_config(unsigned gpio) > int type; > > u = readl(&bank->gpio_config[GPIO_PORT(gpio)]); > - type = (u >> GPIO_BIT(gpio)) & 1; > + type = (u >> GPIO_BIT(gpio)) & 1; > > debug("get_config: port = %d, bit = %d is %s\n", > GPIO_FULLPORT(gpio), GPIO_BIT(gpio), type ? "GPIO" : "SFPIO"); > > - return type; > + return type ? CONFIG_GPIO : CONFIG_SFIO; > } > > -/* Config pin 'gpio' as GPIO or SFPIO, based on 'type' */ > +/* Config pin 'gpio' as GPIO or SFIO, based on 'type' */ > static void set_config(unsigned gpio, int type) > { > struct gpio_ctlr *ctlr = (struct gpio_ctlr *)NV_PA_GPIO_BASE; > @@ -65,7 +70,7 @@ static void set_config(unsigned gpio, int type) > GPIO_FULLPORT(gpio), GPIO_BIT(gpio), type ? "GPIO" : "SFPIO"); > > u = readl(&bank->gpio_config[GPIO_PORT(gpio)]); > - if (type) /* GPIO */ > + if (type != CONFIG_SFIO) > u |= 1 << GPIO_BIT(gpio); > else > u &= ~(1 << GPIO_BIT(gpio)); > @@ -86,7 +91,7 @@ static int get_direction(unsigned gpio) > debug("get_direction: port = %d, bit = %d, %s\n", > GPIO_FULLPORT(gpio), GPIO_BIT(gpio), dir ? "OUT" : "IN"); > > - return dir; > + return dir ? DIRECTION_OUTPUT : DIRECTION_INPUT; > } > > /* Config GPIO pin 'gpio' as input or output (OE) as per 'output' */ > @@ -100,7 +105,7 @@ static void set_direction(unsigned gpio, int output) > GPIO_FULLPORT(gpio), GPIO_BIT(gpio), output ? "OUT" : "IN"); > > u = readl(&bank->gpio_dir_out[GPIO_PORT(gpio)]); > - if (output) > + if (output != DIRECTION_INPUT) > u |= 1 << GPIO_BIT(gpio); > else > u &= ~(1 << GPIO_BIT(gpio)); > @@ -135,7 +140,7 @@ static int tegra_gpio_direction_input(struct udevice *dev, unsigned offset) > struct tegra_port_info *state = dev_get_priv(dev); > > /* Configure GPIO direction as input. */ > - set_direction(state->base_gpio + offset, 0); > + set_direction(state->base_gpio + offset, DIRECTION_INPUT); > > /* Enable the pin as a GPIO */ > set_config(state->base_gpio + offset, 1); > @@ -154,7 +159,7 @@ static int tegra_gpio_direction_output(struct udevice *dev, unsigned offset, > set_level(gpio, value); > > /* Configure GPIO direction as output. */ > - set_direction(gpio, 1); > + set_direction(gpio, DIRECTION_OUTPUT); > > /* Enable the pin as a GPIO */ > set_config(state->base_gpio + offset, 1); > @@ -199,18 +204,18 @@ void gpio_config_table(const struct tegra_gpio_config *config, int len) > for (i = 0; i < len; i++) { > switch (config[i].init) { > case TEGRA_GPIO_INIT_IN: > - set_direction(config[i].gpio, 0); > + set_direction(config[i].gpio, DIRECTION_INPUT); > break; > case TEGRA_GPIO_INIT_OUT0: > set_level(config[i].gpio, 0); > - set_direction(config[i].gpio, 1); > + set_direction(config[i].gpio, DIRECTION_OUTPUT); > break; > case TEGRA_GPIO_INIT_OUT1: > set_level(config[i].gpio, 1); > - set_direction(config[i].gpio, 1); > + set_direction(config[i].gpio, DIRECTION_OUTPUT); > break; > } > - set_config(config[i].gpio, 1); > + set_config(config[i].gpio, CONFIG_GPIO); > } > } > > -- > 1.9.1 > Regards, Simon ^ permalink raw reply [flat|nested] 7+ messages in thread
* [U-Boot] [PATCH 2/2] gpio: tegra: use named constants 2015-10-01 23:00 ` Simon Glass @ 2015-10-01 23:29 ` Stephen Warren 2015-10-03 14:14 ` Simon Glass 0 siblings, 1 reply; 7+ messages in thread From: Stephen Warren @ 2015-10-01 23:29 UTC (permalink / raw) To: u-boot On 10/01/2015 05:00 PM, Simon Glass wrote: > On Friday, 25 September 2015, Stephen Warren <swarren@wwwdotorg.org> wrote: >> From: Stephen Warren <swarren@nvidia.com> >> >> In order to make it clear what the parameters to set_config() and >> set_direction() mean, and similarly for the return values from the >> respective get_*(), define named constants for these values. >> >> Disassembly shows no diff in the generated code, except that the >> order of the code in the branches of tegra_gpio_get_function() gets >> modified without affecting behaviour. >> diff --git a/drivers/gpio/tegra_gpio.c b/drivers/gpio/tegra_gpio.c >> +static const int CONFIG_SFIO = 0; >> +static const int CONFIG_GPIO = 1; >> +static const int DIRECTION_INPUT = 0; >> +static const int DIRECTION_OUTPUT = 1; > > Why not use an enum? I don't think it gives any benefit does it? Doing so would entail 5 extra lines of overhead for the enum { and } lines. I'd want to define two separate enum blocks since I dislike putting logically unrelated enum values into a single enum definition. Even if I didn't do that, it's still 2 lines of useless overhead to add everything into a single enum. ^ permalink raw reply [flat|nested] 7+ messages in thread
* [U-Boot] [PATCH 2/2] gpio: tegra: use named constants 2015-10-01 23:29 ` Stephen Warren @ 2015-10-03 14:14 ` Simon Glass 2015-10-03 19:17 ` Stephen Warren 0 siblings, 1 reply; 7+ messages in thread From: Simon Glass @ 2015-10-03 14:14 UTC (permalink / raw) To: u-boot Hi Stephen, On 2 October 2015 at 00:29, Stephen Warren <swarren@wwwdotorg.org> wrote: > On 10/01/2015 05:00 PM, Simon Glass wrote: >> >> On Friday, 25 September 2015, Stephen Warren <swarren@wwwdotorg.org> >> wrote: >>> >>> From: Stephen Warren <swarren@nvidia.com> >>> >>> In order to make it clear what the parameters to set_config() and >>> set_direction() mean, and similarly for the return values from the >>> respective get_*(), define named constants for these values. >>> >>> Disassembly shows no diff in the generated code, except that the >>> order of the code in the branches of tegra_gpio_get_function() gets >>> modified without affecting behaviour. > > >>> diff --git a/drivers/gpio/tegra_gpio.c b/drivers/gpio/tegra_gpio.c > > >>> +static const int CONFIG_SFIO = 0; >>> +static const int CONFIG_GPIO = 1; >>> +static const int DIRECTION_INPUT = 0; >>> +static const int DIRECTION_OUTPUT = 1; >> >> >> Why not use an enum? > > > I don't think it gives any benefit does it? > > Doing so would entail 5 extra lines of overhead for the enum { and } lines. > I'd want to define two separate enum blocks since I dislike putting > logically unrelated enum values into a single enum definition. Even if I > didn't do that, it's still 2 lines of useless overhead to add everything > into a single enum. It's just odd to use const int instead of enum I think. Particularly as you are using capital letters. Since it's Tegra code I'll leave it up to you. Regards, Simon ^ permalink raw reply [flat|nested] 7+ messages in thread
* [U-Boot] [PATCH 2/2] gpio: tegra: use named constants 2015-10-03 14:14 ` Simon Glass @ 2015-10-03 19:17 ` Stephen Warren 0 siblings, 0 replies; 7+ messages in thread From: Stephen Warren @ 2015-10-03 19:17 UTC (permalink / raw) To: u-boot On 10/03/2015 08:14 AM, Simon Glass wrote: > Hi Stephen, > > On 2 October 2015 at 00:29, Stephen Warren <swarren@wwwdotorg.org> wrote: >> On 10/01/2015 05:00 PM, Simon Glass wrote: >>> >>> On Friday, 25 September 2015, Stephen Warren <swarren@wwwdotorg.org> >>> wrote: >>>> >>>> From: Stephen Warren <swarren@nvidia.com> >>>> >>>> In order to make it clear what the parameters to set_config() and >>>> set_direction() mean, and similarly for the return values from the >>>> respective get_*(), define named constants for these values. >>>> >>>> Disassembly shows no diff in the generated code, except that the >>>> order of the code in the branches of tegra_gpio_get_function() gets >>>> modified without affecting behaviour. >> >> >>>> diff --git a/drivers/gpio/tegra_gpio.c b/drivers/gpio/tegra_gpio.c >> >> >>>> +static const int CONFIG_SFIO = 0; >>>> +static const int CONFIG_GPIO = 1; >>>> +static const int DIRECTION_INPUT = 0; >>>> +static const int DIRECTION_OUTPUT = 1; >>> >>> >>> Why not use an enum? >> >> >> I don't think it gives any benefit does it? >> >> Doing so would entail 5 extra lines of overhead for the enum { and } lines. >> I'd want to define two separate enum blocks since I dislike putting >> logically unrelated enum values into a single enum definition. Even if I >> didn't do that, it's still 2 lines of useless overhead to add everything >> into a single enum. > > It's just odd to use const int instead of enum I think. What makes it odd? ^ permalink raw reply [flat|nested] 7+ messages in thread
* [U-Boot] [PATCH 1/2] gpio: tegra: remove unused type 2015-09-25 16:44 [U-Boot] [PATCH 1/2] gpio: tegra: remove unused type Stephen Warren 2015-09-25 16:44 ` [U-Boot] [PATCH 2/2] gpio: tegra: use named constants Stephen Warren @ 2015-10-01 23:00 ` Simon Glass 1 sibling, 0 replies; 7+ messages in thread From: Simon Glass @ 2015-10-01 23:00 UTC (permalink / raw) To: u-boot On Friday, 25 September 2015, Stephen Warren <swarren@wwwdotorg.org> wrote: > > From: Stephen Warren <swarren@nvidia.com> > > These enum values aren't used anywhere. Remove them. > > Signed-off-by: Stephen Warren <swarren@nvidia.com> > --- > This series depends on my previous "ARM: tegra: don't enable GPIOs > until direction is set" > > drivers/gpio/tegra_gpio.c | 7 ------- > 1 file changed, 7 deletions(-) > Reviewed-by: Simon Glass <sjg@chromium.org> ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2015-10-03 19:17 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-09-25 16:44 [U-Boot] [PATCH 1/2] gpio: tegra: remove unused type Stephen Warren 2015-09-25 16:44 ` [U-Boot] [PATCH 2/2] gpio: tegra: use named constants Stephen Warren 2015-10-01 23:00 ` Simon Glass 2015-10-01 23:29 ` Stephen Warren 2015-10-03 14:14 ` Simon Glass 2015-10-03 19:17 ` Stephen Warren 2015-10-01 23:00 ` [U-Boot] [PATCH 1/2] gpio: tegra: remove unused type Simon Glass
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox