| Message ID | 1321884585-12501-1-git-send-email-fabio.estevam@freescale.com |
|---|---|
| State | Rejected |
| Delegated to: | Stefano Babic |
| Headers | show |
> Configure the pins as GPIOs prior to using gpio_get_value > > Cc: Marek Vasut <marek.vasut@gmail.com> > Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com> > --- > board/efikamx/efikamx.c | 13 +++++++++---- > 1 files changed, 9 insertions(+), 4 deletions(-) > > diff --git a/board/efikamx/efikamx.c b/board/efikamx/efikamx.c > index 3d2cc1a..b911891 100644 > --- a/board/efikamx/efikamx.c > +++ b/board/efikamx/efikamx.c > @@ -308,10 +308,14 @@ struct fsl_esdhc_cfg esdhc_cfg[2] = { > > static inline uint32_t efika_mmc_cd(void) > { > - if (machine_is_efikamx()) > + if (machine_is_efikamx()) { > + mxc_request_iomux(MX51_PIN_GPIO1_0, IOMUX_CONFIG_ALT1); > return MX51_PIN_GPIO1_0; > - else > + > + } else { > + mxc_request_iomux(MX51_PIN_EIM_CS2, IOMUX_CONFIG_ALT1); > return MX51_PIN_EIM_CS2; > + } > } > > int board_mmc_getcd(u8 *absent, struct mmc *mmc) > @@ -321,9 +325,10 @@ int board_mmc_getcd(u8 *absent, struct mmc *mmc) > > if (cfg->esdhc_base == MMC_SDHC1_BASE_ADDR) > *absent = gpio_get_value(IOMUX_TO_GPIO(cd)); > - else > + else { > + mxc_request_iomux(MX51_PIN_GPIO1_8, IOMUX_CONFIG_ALT0); > *absent = gpio_get_value(IOMUX_TO_GPIO(MX51_PIN_GPIO1_8)); > - > + } > return 0; > } NAK. There should be some common function for setting up iomux of those pins. You souldn't set it in repeatedly called functions. M
On Monday 21 November 2011 09:20:49 Marek Vasut wrote: > > Configure the pins as GPIOs prior to using gpio_get_value > > > > + else { > > + mxc_request_iomux(MX51_PIN_GPIO1_8, IOMUX_CONFIG_ALT0); > > *absent = gpio_get_value(IOMUX_TO_GPIO(MX51_PIN_GPIO1_8)); > > - > > + } > > return 0; > > } > > NAK. There should be some common function for setting up iomux of those > pins. You souldn't set it in repeatedly called functions. that's what gpio_request() is for -mike
> On Monday 21 November 2011 09:20:49 Marek Vasut wrote: > > > Configure the pins as GPIOs prior to using gpio_get_value > > > > > > + else { > > > + mxc_request_iomux(MX51_PIN_GPIO1_8, IOMUX_CONFIG_ALT0); > > > > > > *absent = gpio_get_value(IOMUX_TO_GPIO(MX51_PIN_GPIO1_8)); > > > > > > - > > > + } > > > > > > return 0; > > > > > > } > > > > NAK. There should be some common function for setting up iomux of those > > pins. You souldn't set it in repeatedly called functions. > > that's what gpio_request() is for I mean in efika.c ... there should be a common place for these iomux configurations being done. This is unrelated to gpio ... > -mike
On Monday 21 November 2011 14:29:50 Marek Vasut wrote: > > On Monday 21 November 2011 09:20:49 Marek Vasut wrote: > > > > Configure the pins as GPIOs prior to using gpio_get_value > > > > > > > > + else { > > > > + mxc_request_iomux(MX51_PIN_GPIO1_8, IOMUX_CONFIG_ALT0); > > > > *absent = gpio_get_value(IOMUX_TO_GPIO(MX51_PIN_GPIO1_8)); > > > > - > > > > + } > > > > > > NAK. There should be some common function for setting up iomux of those > > > pins. You souldn't set it in repeatedly called functions. > > > > that's what gpio_request() is for > > I mean in efika.c ... there should be a common place for these iomux > configurations being done. This is unrelated to gpio ... not really ... imo, if someone does gpio_request(PIN), the gpio core should take care of putting it into GPIO mode. people shouldn't have to pinmux_request(PIN, GPIO_MODE) before doing gpio_request(PIN). -mike
> On Monday 21 November 2011 14:29:50 Marek Vasut wrote: > > > On Monday 21 November 2011 09:20:49 Marek Vasut wrote: > > > > > Configure the pins as GPIOs prior to using gpio_get_value > > > > > > > > > > + else { > > > > > + mxc_request_iomux(MX51_PIN_GPIO1_8, IOMUX_CONFIG_ALT0); > > > > > > > > > > *absent = gpio_get_value(IOMUX_TO_GPIO(MX51_PIN_GPIO1_8)); > > > > > > > > > > - > > > > > + } > > > > > > > > NAK. There should be some common function for setting up iomux of > > > > those pins. You souldn't set it in repeatedly called functions. > > > > > > that's what gpio_request() is for > > > > I mean in efika.c ... there should be a common place for these iomux > > configurations being done. This is unrelated to gpio ... > > not really ... imo, if someone does gpio_request(PIN), the gpio core should > take care of putting it into GPIO mode. people shouldn't have to > pinmux_request(PIN, GPIO_MODE) before doing gpio_request(PIN). Of course ... considering there's always one correct setting for the pin to be in GPIO mode, which I suspect might not be completely true today anymore.
On Monday 21 November 2011 15:53:45 Marek Vasut wrote: > > On Monday 21 November 2011 14:29:50 Marek Vasut wrote: > > > > On Monday 21 November 2011 09:20:49 Marek Vasut wrote: > > > > > > Configure the pins as GPIOs prior to using gpio_get_value > > > > > > > > > > > > + else { > > > > > > + mxc_request_iomux(MX51_PIN_GPIO1_8, IOMUX_CONFIG_ALT0); > > > > > > *absent = gpio_get_value(IOMUX_TO_GPIO(MX51_PIN_GPIO1_8)); > > > > > > - > > > > > > + } > > > > > > > > > > NAK. There should be some common function for setting up iomux of > > > > > those pins. You souldn't set it in repeatedly called functions. > > > > > > > > that's what gpio_request() is for > > > > > > I mean in efika.c ... there should be a common place for these iomux > > > configurations being done. This is unrelated to gpio ... > > > > not really ... imo, if someone does gpio_request(PIN), the gpio core > > should take care of putting it into GPIO mode. people shouldn't have to > > pinmux_request(PIN, GPIO_MODE) before doing gpio_request(PIN). > > Of course ... considering there's always one correct setting for the pin to > be in GPIO mode, which I suspect might not be completely true today > anymore. i find it hard to envision a pinmux system where individual pins would have different pinmux configurations to get it into GPIO mode. probably be saner to have gpio_request() do the right thing and wait for someone to come forward with the unusual setup -- worry about it then. -mike
On 21/11/2011 22:22, Mike Frysinger wrote: >> Of course ... considering there's always one correct setting for >> the pin to be in GPIO mode, which I suspect might not be >> completely true today anymore. > > i find it hard to envision a pinmux system where individual pins > would have different pinmux configurations to get it into GPIO > mode. probably be saner to have gpio_request() do the right thing > and wait for someone to come forward with the unusual setup -- > worry about it then. In fact it would be nicer if gpio_request() takes care of the pinmux, in the way I can see on the davinci SOCs. However, on the IMXs a single GPIO can be connected (not at the same time, of course) to different PADs, depending on a general setup (GPR register) or if the daisy chain in the multiplexer is activated. Calling gpio_request and passing only the gpio number is not enough on the IMXs to find where the GPIO is routed. The second point I will arise is that, mainly due to the different internal layout but also for historical reasons, the setup and the provided function for the multiplexer is very different among the SOCs. Only mx35 and mx5 expone the same interface (mxc_request_iomux), while mx31/mx25/mx27/mx28 have its own. Because we use and we want to use the GPIO framework, the gpio driver, common to all IMX SOCs, should be able to set up the multiplexer independently from the SOC type, that means we should have the same interface for the multiplexer, and we have not (yet ?). Stefano
On Tuesday 22 November 2011 03:15:47 Stefano Babic wrote: > On 21/11/2011 22:22, Mike Frysinger wrote: > >> Of course ... considering there's always one correct setting for > >> the pin to be in GPIO mode, which I suspect might not be > >> completely true today anymore. > > > > i find it hard to envision a pinmux system where individual pins > > would have different pinmux configurations to get it into GPIO > > mode. probably be saner to have gpio_request() do the right thing > > and wait for someone to come forward with the unusual setup -- > > worry about it then. > > In fact it would be nicer if gpio_request() takes care of the pinmux, > in the way I can see on the davinci SOCs. However, on the IMXs a > single GPIO can be connected (not at the same time, of course) to > different PADs, depending on a general setup (GPR register) or if the > daisy chain in the multiplexer is activated. if it's different physical pins, then perhaps it should be different GPIO numbers ? > The second point I will arise is that, mainly due to the different > internal layout but also for historical reasons, the setup and the > provided function for the multiplexer is very different among the SOCs. > > Only mx35 and mx5 expone the same interface (mxc_request_iomux), while > mx31/mx25/mx27/mx28 have its own. Because we use and we want to use > the GPIO framework, the gpio driver, common to all IMX SOCs, should be > able to set up the multiplexer independently from the SOC type, that > means we should have the same interface for the multiplexer, and we > have not (yet ?). this is shaking out in Linux with the pinmux framework, so probably best to grit our teeth until that's done and then adopt what they implement. -mike
Hi, On 11/22/11 23:07, Mike Frysinger wrote: > On Tuesday 22 November 2011 03:15:47 Stefano Babic wrote: >> On 21/11/2011 22:22, Mike Frysinger wrote: >>>> Of course ... considering there's always one correct setting for >>>> the pin to be in GPIO mode, which I suspect might not be >>>> completely true today anymore. >>> >>> i find it hard to envision a pinmux system where individual pins >>> would have different pinmux configurations to get it into GPIO >>> mode. probably be saner to have gpio_request() do the right thing >>> and wait for someone to come forward with the unusual setup -- >>> worry about it then. >> >> In fact it would be nicer if gpio_request() takes care of the pinmux, >> in the way I can see on the davinci SOCs. However, on the IMXs a >> single GPIO can be connected (not at the same time, of course) to >> different PADs, depending on a general setup (GPR register) or if the >> daisy chain in the multiplexer is activated. > > if it's different physical pins, then perhaps it should be different GPIO > numbers ? I think, currently, you are right, but if we look in some other AF (Alternate Functions) (not GPIO), same AF can be available on different physical pins. This means that in theory, same GPIO number can also be connected to different physical pins. I think it should be board controllable instead of gpiolib hard coded, or of course pinmux controlled (which is board controllable). > >> The second point I will arise is that, mainly due to the different >> internal layout but also for historical reasons, the setup and the >> provided function for the multiplexer is very different among the SOCs. >> >> Only mx35 and mx5 expone the same interface (mxc_request_iomux), while >> mx31/mx25/mx27/mx28 have its own. Because we use and we want to use >> the GPIO framework, the gpio driver, common to all IMX SOCs, should be >> able to set up the multiplexer independently from the SOC type, that >> means we should have the same interface for the multiplexer, and we >> have not (yet ?). > > this is shaking out in Linux with the pinmux framework, so probably best to > grit our teeth until that's done and then adopt what they implement. This is probably the best way and correct me if I'm wrong, this means stick with board control over the pinmux instead of gpiolib.
On 22/11/2011 22:07, Mike Frysinger wrote: > > if it's different physical pins, then perhaps it should be > different GPIO numbers ? No, they are not. Depending on the "daisy chain" functionality and based on different alternate function, a single GPIO can be routed to different physical pins. > this is shaking out in Linux with the pinmux framework, so probably > best to grit our teeth until that's done and then adopt what they > implement. Agree. Best regards, Stefano Babic
On Wednesday 23 November 2011 02:02:25 Igor Grinberg wrote: > On 11/22/11 23:07, Mike Frysinger wrote: > > On Tuesday 22 November 2011 03:15:47 Stefano Babic wrote: > >> On 21/11/2011 22:22, Mike Frysinger wrote: > >>>> Of course ... considering there's always one correct setting for > >>>> the pin to be in GPIO mode, which I suspect might not be > >>>> completely true today anymore. > >>> > >>> i find it hard to envision a pinmux system where individual pins > >>> would have different pinmux configurations to get it into GPIO > >>> mode. probably be saner to have gpio_request() do the right thing > >>> and wait for someone to come forward with the unusual setup -- > >>> worry about it then. > >> > >> In fact it would be nicer if gpio_request() takes care of the pinmux, > >> in the way I can see on the davinci SOCs. However, on the IMXs a > >> single GPIO can be connected (not at the same time, of course) to > >> different PADs, depending on a general setup (GPR register) or if the > >> daisy chain in the multiplexer is activated. > > > > if it's different physical pins, then perhaps it should be different GPIO > > numbers ? > > I think, currently, you are right, but if we look in some other > AF (Alternate Functions) (not GPIO), same AF can be available on > different physical pins. This means that in theory, same GPIO number > can also be connected to different physical pins. > I think it should be board controllable instead of gpiolib hard coded, > or of course pinmux controlled (which is board controllable). i understand AF for peripheral pins, but i'm not seeing the same-gpio-number- for-different-pins part. > >> The second point I will arise is that, mainly due to the different > >> internal layout but also for historical reasons, the setup and the > >> provided function for the multiplexer is very different among the SOCs. > >> > >> Only mx35 and mx5 expone the same interface (mxc_request_iomux), while > >> mx31/mx25/mx27/mx28 have its own. Because we use and we want to use > >> the GPIO framework, the gpio driver, common to all IMX SOCs, should be > >> able to set up the multiplexer independently from the SOC type, that > >> means we should have the same interface for the multiplexer, and we > >> have not (yet ?). > > > > this is shaking out in Linux with the pinmux framework, so probably best > > to grit our teeth until that's done and then adopt what they implement. > > This is probably the best way and correct me if I'm wrong, > this means stick with board control over the pinmux > instead of gpiolib. *shrug* whatever the maintainer of the gpio/drivers want to do. as long as it's not common code, i won't worry about it for now. -mike
On Wednesday 23 November 2011 03:19:48 Stefano Babic wrote: > On 22/11/2011 22:07, Mike Frysinger wrote: > > if it's different physical pins, then perhaps it should be > > different GPIO numbers ? > > No, they are not. Depending on the "daisy chain" functionality and > based on different alternate function, a single GPIO can be routed to > different physical pins. the GPIO # can represent the AF as well as the GPIO itself. this is what bit shifts and masks are good at. there's no requirement that the GPIO #'s be contiguous, or have a specific range. -mike
diff --git a/board/efikamx/efikamx.c b/board/efikamx/efikamx.c index 3d2cc1a..b911891 100644 --- a/board/efikamx/efikamx.c +++ b/board/efikamx/efikamx.c @@ -308,10 +308,14 @@ struct fsl_esdhc_cfg esdhc_cfg[2] = { static inline uint32_t efika_mmc_cd(void) { - if (machine_is_efikamx()) + if (machine_is_efikamx()) { + mxc_request_iomux(MX51_PIN_GPIO1_0, IOMUX_CONFIG_ALT1); return MX51_PIN_GPIO1_0; - else + + } else { + mxc_request_iomux(MX51_PIN_EIM_CS2, IOMUX_CONFIG_ALT1); return MX51_PIN_EIM_CS2; + } } int board_mmc_getcd(u8 *absent, struct mmc *mmc) @@ -321,9 +325,10 @@ int board_mmc_getcd(u8 *absent, struct mmc *mmc) if (cfg->esdhc_base == MMC_SDHC1_BASE_ADDR) *absent = gpio_get_value(IOMUX_TO_GPIO(cd)); - else + else { + mxc_request_iomux(MX51_PIN_GPIO1_8, IOMUX_CONFIG_ALT0); *absent = gpio_get_value(IOMUX_TO_GPIO(MX51_PIN_GPIO1_8)); - + } return 0; }
Configure the pins as GPIOs prior to using gpio_get_value Cc: Marek Vasut <marek.vasut@gmail.com> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com> --- board/efikamx/efikamx.c | 13 +++++++++---- 1 files changed, 9 insertions(+), 4 deletions(-)