| Message ID | DA037FBB-56A7-4379-88F0-650367BBB822@martin.sperl.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Sun, Mar 29, 2015 at 04:03:28PM +0200, Martin Sperl wrote: > A variation of this probably should get moved into spi_register_master > to fix this for other spi-controller as well. Yes, this is the wrong place to solve this.
> On 30.03.2015, at 06:28, Mark Brown <broonie@kernel.org> wrote: > > On Sun, Mar 29, 2015 at 04:03:28PM +0200, Martin Sperl wrote: > >> A variation of this probably should get moved into spi_register_master >> to fix this for other spi-controller as well. > > Yes, this is the wrong place to solve this. So should we move that into spi_register_master? My question for completeness remains: how do we get the devices from dt or whatever while we are still registering the master? Also I am still missing a bit of the big picture and vision how to implement it taking all different HW/setup cases into account. That is why I have put it as an RFC - mostly to get the issue some exposure and some ideas. -- To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 03/30/2015 01:25 PM, Martin Sperl wrote: > >> On 30.03.2015, at 06:28, Mark Brown <broonie@kernel.org> wrote: >> >> On Sun, Mar 29, 2015 at 04:03:28PM +0200, Martin Sperl wrote: >> >>> A variation of this probably should get moved into spi_register_master >>> to fix this for other spi-controller as well. >> >> Yes, this is the wrong place to solve this. > > So should we move that into spi_register_master? > My question for completeness remains: how do we get the devices > from dt or whatever while we are still registering the master? > > Also I am still missing a bit of the big picture and vision how > to implement it taking all different HW/setup cases into account. > > That is why I have put it as an RFC - mostly to get the issue some > exposure and some ideas. I suspect the solution is simply not to have the pinctrl node set the CS pins to GPIO_OUT, but rather rely on gpio_request() -> bcm2835_gpio_request() -> bcm2835_pmx_gpio_set_direction() setting up the pinmux function for any GPIOs that are requested. Presumably, the CS GPIO for each chip is only requested when the DT node for the SPI device (not SPI controller device) has been parsed, so the correct GPIO CS value can be passed to gpio_request()/gpio_direction_output()? -- To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> On 31.03.2015, at 05:35, Stephen Warren <swarren@wwwdotorg.org> wrote: > > I suspect the solution is simply not to have the pinctrl node set the CS > pins to GPIO_OUT, but rather rely on gpio_request() -> > bcm2835_gpio_request() -> bcm2835_pmx_gpio_set_direction() setting up > the pinmux function for any GPIOs that are requested. Presumably, the CS > GPIO for each chip is only requested when the DT node for the SPI device > (not SPI controller device) has been parsed, so the correct GPIO CS > value can be passed to gpio_request()/gpio_direction_output()? I will investigate that and create a patch to spi_register_master that does that for any GPIO-chipselect - this still leaves the question of how to iterate over all the configured devices early to detect if they hace cspol set and then setting it low instead of high. Martin -- To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/spi/spi-bcm2835.c b/drivers/spi/spi-bcm2835.c index 1d731b8..b6825a4 100644 --- a/drivers/spi/spi-bcm2835.c +++ b/drivers/spi/spi-bcm2835.c @@ -349,6 +349,26 @@ static int bcm2835_spi_setup(struct spi_device *spi) return -EINVAL; } +static void bcm2835_spi_initialsetup_gpio(struct spi_master *master) +{ + int i; + bool level; + + if (!master->cs_gpios) + return; + + for (i = 0; i < master->num_chipselect; i++) { + if (gpio_is_valid(master->cs_gpios[i])) { + /* actually we need to figure out the + * configured CSPOL of that device here, + * not sure how to do that... + */ + level = true; + gpio_set_value(master->cs_gpios[i], level); + } + } +} + static int bcm2835_spi_probe(struct platform_device *pdev) { struct spi_master *master; @@ -414,6 +434,9 @@ static int bcm2835_spi_probe(struct platform_device *pdev) goto out_clk_disable; } + /* set GPIO-CS to disabled */ + bcm2835_spi_initialsetup_gpio(master); + return 0; out_clk_disable: