* [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 an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.