| Message ID | 1456510756-15337-2-git-send-email-eric@anholt.net |
|---|---|
| State | New |
| Headers | show |
On 02/26/2016 11:19 AM, Eric Anholt wrote: > The BCM2835-ARM-Peripherals.pdf documentation specifies what the > function selects do for the pins, and there are a bunch of obvious > groupings to be made. With these created, we'll be able to replace > bcm2835-rpi.dtsi's main "set all of these pins to alt0" with > references to specific groups we want enabled. > diff --git a/arch/arm/boot/dts/bcm283x.dtsi b/arch/arm/boot/dts/bcm283x.dtsi > + spi0_gpio7: spi0_gpio7 { > + brcm,pins = <7 8 9 10 11>; > + brcm,function = <BCM2835_FSEL_ALT0>; > + }; This is too many pins. - It includes both MOSI and MISO, although a particular use-case may only use 1 of those. - It includes both chip-select signals, whereas a particular use-case may use 0, 1, or 2 of those. This is especially true since IIRC the mainline bcm283x SPI driver wants to only use GPIOs for chip-selects, not SPI-controller-generated chip-select signals, to avoid some issues with the HW generation of these signals. I believe a similar comment applies to other SPI nodes too. > + pcm_gpio18: pcm_gpio18 { > + brcm,pins = <18 19 20 21>; > + brcm,function = <BCM2835_FSEL_ALT0>; > + }; Here too, I wonder if some people might want only one of DIN/DOUT and not both? > + uart1_gpio36: uart1_gpio36 { > + brcm,pins = <36 37 38 39>; > + brcm,function = <BCM2835_FSEL_ALT2>; > + }; Similarly, I think for UARTS, TX/RX and RTS/CTS should always be in different nodes so people can choose 2- or 4-wire mode. Most of the UART nodes are already split like this, but this one isn't. > + emmc_gpio22: emmc_gpio22 { > + brcm,pins = <22 23 24 25 26 27>; > + brcm,function = <BCM2835_FSEL_ALT3>; > + }; 1-wire (1 data wire, plus CLK/CMD) eMMC is possible in theory, although I don't know whether it makes sense to support this? -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Stephen Warren <swarren@wwwdotorg.org> writes: > On 02/26/2016 11:19 AM, Eric Anholt wrote: >> The BCM2835-ARM-Peripherals.pdf documentation specifies what the >> function selects do for the pins, and there are a bunch of obvious >> groupings to be made. With these created, we'll be able to replace >> bcm2835-rpi.dtsi's main "set all of these pins to alt0" with >> references to specific groups we want enabled. > >> diff --git a/arch/arm/boot/dts/bcm283x.dtsi b/arch/arm/boot/dts/bcm283x.dtsi > >> + spi0_gpio7: spi0_gpio7 { >> + brcm,pins = <7 8 9 10 11>; >> + brcm,function = <BCM2835_FSEL_ALT0>; >> + }; > > This is too many pins. > > - It includes both MOSI and MISO, although a particular use-case may > only use 1 of those. > > - It includes both chip-select signals, whereas a particular use-case > may use 0, 1, or 2 of those. This is especially true since IIRC the > mainline bcm283x SPI driver wants to only use GPIOs for chip-selects, > not SPI-controller-generated chip-select signals, to avoid some issues > with the HW generation of these signals. > > > I believe a similar comment applies to other SPI nodes too. > >> + pcm_gpio18: pcm_gpio18 { >> + brcm,pins = <18 19 20 21>; >> + brcm,function = <BCM2835_FSEL_ALT0>; >> + }; > > Here too, I wonder if some people might want only one of DIN/DOUT and > not both? > >> + uart1_gpio36: uart1_gpio36 { >> + brcm,pins = <36 37 38 39>; >> + brcm,function = <BCM2835_FSEL_ALT2>; >> + }; > > Similarly, I think for UARTS, TX/RX and RTS/CTS should always be in > different nodes so people can choose 2- or 4-wire mode. Most of the UART > nodes are already split like this, but this one isn't. > >> + emmc_gpio22: emmc_gpio22 { >> + brcm,pins = <22 23 24 25 26 27>; >> + brcm,function = <BCM2835_FSEL_ALT3>; >> + }; > > 1-wire (1 data wire, plus CLK/CMD) eMMC is possible in theory, although > I don't know whether it makes sense to support this? Nothing here precludes making alternative pin groups for special situations like you're bringing up here. I'm just trying to bring sanity to the giant lists of pins we have currently, that happen to correspond to these. Of your suggestions, making uart1_gpio36 split out cts/rts like the rest makes a lot of sense to me. Of the others, they seem like speculation more than "we should fix this because it's not what people want." Can you provide specific feedback of what you'd like changed to get an Ack?
On 03/03/2016 03:23 PM, Eric Anholt wrote: > Stephen Warren <swarren@wwwdotorg.org> writes: > >> On 02/26/2016 11:19 AM, Eric Anholt wrote: >>> The BCM2835-ARM-Peripherals.pdf documentation specifies what the >>> function selects do for the pins, and there are a bunch of obvious >>> groupings to be made. With these created, we'll be able to replace >>> bcm2835-rpi.dtsi's main "set all of these pins to alt0" with >>> references to specific groups we want enabled. >> >>> diff --git a/arch/arm/boot/dts/bcm283x.dtsi b/arch/arm/boot/dts/bcm283x.dtsi >> >>> + spi0_gpio7: spi0_gpio7 { >>> + brcm,pins = <7 8 9 10 11>; >>> + brcm,function = <BCM2835_FSEL_ALT0>; >>> + }; >> >> This is too many pins. >> >> - It includes both MOSI and MISO, although a particular use-case may >> only use 1 of those. >> >> - It includes both chip-select signals, whereas a particular use-case >> may use 0, 1, or 2 of those. This is especially true since IIRC the >> mainline bcm283x SPI driver wants to only use GPIOs for chip-selects, >> not SPI-controller-generated chip-select signals, to avoid some issues >> with the HW generation of these signals. >> >> >> I believe a similar comment applies to other SPI nodes too. >> >>> + pcm_gpio18: pcm_gpio18 { >>> + brcm,pins = <18 19 20 21>; >>> + brcm,function = <BCM2835_FSEL_ALT0>; >>> + }; >> >> Here too, I wonder if some people might want only one of DIN/DOUT and >> not both? >> >>> + uart1_gpio36: uart1_gpio36 { >>> + brcm,pins = <36 37 38 39>; >>> + brcm,function = <BCM2835_FSEL_ALT2>; >>> + }; >> >> Similarly, I think for UARTS, TX/RX and RTS/CTS should always be in >> different nodes so people can choose 2- or 4-wire mode. Most of the UART >> nodes are already split like this, but this one isn't. >> >>> + emmc_gpio22: emmc_gpio22 { >>> + brcm,pins = <22 23 24 25 26 27>; >>> + brcm,function = <BCM2835_FSEL_ALT3>; >>> + }; >> >> 1-wire (1 data wire, plus CLK/CMD) eMMC is possible in theory, although >> I don't know whether it makes sense to support this? > > Nothing here precludes making alternative pin groups for special > situations like you're bringing up here. I'm just trying to bring > sanity to the giant lists of pins we have currently, that happen to > correspond to these. > > Of your suggestions, making uart1_gpio36 split out cts/rts like the rest > makes a lot of sense to me. Of the others, they seem like speculation > more than "we should fix this because it's not what people want." Can > you provide specific feedback of what you'd like changed to get an Ack? All of the points I raised should be fixed. I don't believe any of the groups that affect more than minimal sets of pins are useful. Indeed, using groups at all is rather tenuous; it'd be far better to list the precise sets of pins only as and when they're used. -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> On 03.03.2016, at 22:20, Stephen Warren <swarren@wwwdotorg.org> wrote: > > On 02/26/2016 11:19 AM, Eric Anholt wrote: >> The BCM2835-ARM-Peripherals.pdf documentation specifies what the >> function selects do for the pins, and there are a bunch of obvious >> groupings to be made. With these created, we'll be able to replace >> bcm2835-rpi.dtsi's main "set all of these pins to alt0" with >> references to specific groups we want enabled. > >> diff --git a/arch/arm/boot/dts/bcm283x.dtsi b/arch/arm/boot/dts/bcm283x.dtsi > >> + spi0_gpio7: spi0_gpio7 { >> + brcm,pins = <7 8 9 10 11>; >> + brcm,function = <BCM2835_FSEL_ALT0>; >> + }; > > This is too many pins. > > - It includes both MOSI and MISO, although a particular use-case may only use 1 of those. > > - It includes both chip-select signals, whereas a particular use-case may use 0, 1, or 2 of those. This is especially true since IIRC the mainline bcm283x SPI driver wants to only use GPIOs for chip-selects, not SPI-controller-generated chip-select signals, to avoid some issues with the HW generation of these signals. That is true: the spi-bcm2835 driver requires GPIO usage for chip-select to make all those latency optimizations work (but also to avoid some spi-dma issues). The reason behind it is that there are observed short term “glitches” on native CS whenever the SPI control register is touched - even with identical values. And GPIO controlled CS solves this issue (and Mark Brown said that the GPIO-cs interface is now preferred anyway - hence the auxiliary spi only implement gpio-cs and requires the CS set as OUTPUT, but unlike the main spi this does not have “remapping” support for legacy device-trees (as there never was a driver-version that supported native-cs). Maybe split the SPI-portion into 2 sections: * the SCK, MOSI, MISO (pin 9 to 11) with ALT_0 * the CS GPIOs (standard pins are 7 and 8) with OUTPUT. That way it is easy to override only this section (plus the gpio-cs property inside the spi node) to extend the number of chip selects or use different mappings. > > I believe a similar comment applies to other SPI nodes too. I guess the same “splitting” approach should be taken here as well...-- To unsubscribe from this list: send the line "unsubscribe linux-gpio" 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/arch/arm/boot/dts/bcm283x.dtsi b/arch/arm/boot/dts/bcm283x.dtsi index 8aaf193..e91198e 100644 --- a/arch/arm/boot/dts/bcm283x.dtsi +++ b/arch/arm/boot/dts/bcm283x.dtsi @@ -110,6 +110,176 @@ interrupt-controller; #interrupt-cells = <2>; + + /* Defines pin muxing groups according to + * BCM2835-ARM-Peripherals.pdf page 102. + * + * While each pin can have its mux selected + * for various functions individually, some + * groups only make sense to switch to a + * particular function together. + */ + i2c0_gpio0: i2c0_gpio0 { + brcm,pins = <0 1>; + brcm,function = <BCM2835_FSEL_ALT0>; + }; + i2c1_gpio2: i2c1_gpio2 { + brcm,pins = <2 3>; + brcm,function = <BCM2835_FSEL_ALT0>; + }; + gpclk0_gpio4: gpclk0_gpio4 { + brcm,pins = <4>; + brcm,function = <BCM2835_FSEL_ALT0>; + }; + gpclk1_gpio5: gpclk1_gpio5 { + brcm,pins = <5>; + brcm,function = <BCM2835_FSEL_ALT0>; + }; + gpclk2_gpio6: gpclk2_gpio6 { + brcm,pins = <6>; + brcm,function = <BCM2835_FSEL_ALT0>; + }; + spi0_gpio7: spi0_gpio7 { + brcm,pins = <7 8 9 10 11>; + brcm,function = <BCM2835_FSEL_ALT0>; + }; + pwm0_gpio12: pwm0_gpio12 { + brcm,pins = <12>; + brcm,function = <BCM2835_FSEL_ALT0>; + }; + pwm1_gpio13: pwm1_gpio13 { + brcm,pins = <13>; + brcm,function = <BCM2835_FSEL_ALT0>; + }; + uart0_gpio14: uart0_gpio14 { + brcm,pins = <14 15>; + brcm,function = <BCM2835_FSEL_ALT0>; + }; + pcm_gpio18: pcm_gpio18 { + brcm,pins = <18 19 20 21>; + brcm,function = <BCM2835_FSEL_ALT0>; + }; + i2c0_gpio32: i2c0_gpio32 { + brcm,pins = <32 34>; + brcm,function = <BCM2835_FSEL_ALT0>; + }; + spio0_gpio35: spio0_gpio35 { + brcm,pins = <35 36 37 38 39>; + brcm,function = <BCM2835_FSEL_ALT0>; + }; + pwm0_gpio40: pwm0_gpio40 { + brcm,pins = <40>; + brcm,function = <BCM2835_FSEL_ALT0>; + }; + pwm1_gpio41: pwm1_gpio41 { + brcm,pins = <41>; + brcm,function = <BCM2835_FSEL_ALT0>; + }; + gpclk1_gpio42: gpclk1_gpio42 { + brcm,pins = <42>; + brcm,function = <BCM2835_FSEL_ALT0>; + }; + gpclk2_gpio43: gpclk2_gpio43 { + brcm,pins = <43>; + brcm,function = <BCM2835_FSEL_ALT0>; + }; + gpclk1_gpio44: gpclk1_gpio44 { + brcm,pins = <44>; + brcm,function = <BCM2835_FSEL_ALT0>; + }; + pwm1_gpio45: pwm1_gpio45 { + brcm,pins = <45>; + brcm,function = <BCM2835_FSEL_ALT0>; + }; + i2c0_gpio44: i2c0_gpio44 { + brcm,pins = <44 45>; + brcm,function = <BCM2835_FSEL_ALT1>; + }; + pcm_gpio28: pcm_gpio28 { + brcm,pins = <28 29 30 31>; + brcm,function = <BCM2835_FSEL_ALT2>; + }; + uart1_gpio36: uart1_gpio36 { + brcm,pins = <36 37 38 39>; + brcm,function = <BCM2835_FSEL_ALT2>; + }; + i2c1_gpio44: i2c1_gpio44 { + brcm,pins = <44 45>; + brcm,function = <BCM2835_FSEL_ALT2>; + }; + /* Separate from the uart0_gpio14 group + * because it conflicts with spi1_gpio16, and + * people often run uart0 on the two pins + * without flow contrl. + */ + uart0_ctsrts_gpio16: uart0_ctsrts_gpio16 { + brcm,pins = <16 17>; + brcm,function = <BCM2835_FSEL_ALT3>; + }; + i2c_slave_gpio18: i2c_slave_gpio18 { + brcm,pins = <18 19 20 21>; + brcm,function = <BCM2835_FSEL_ALT3>; + }; + emmc_gpio22: emmc_gpio22 { + brcm,pins = <22 23 24 25 26 27>; + brcm,function = <BCM2835_FSEL_ALT3>; + }; + uart0_gpio30: uart0_gpio30 { + brcm,pins = <30 31>; + brcm,function = <BCM2835_FSEL_ALT3>; + }; + uart0_ctsrts_gpio32: uart0_ctsrts_gpio32 { + brcm,pins = <32 33>; + brcm,function = <BCM2835_FSEL_ALT3>; + }; + spi1_gpio16: spi1_gpio16 { + brcm,pins = <16 17 18 19 20 21>; + brcm,function = <BCM2835_FSEL_ALT4>; + }; + jtag_gpio22: jtag_gpio22 { + brcm,pins = <22 23 24 25 26 27>; + brcm,function = <BCM2835_FSEL_ALT4>; + }; + spi2_gpio40: spi2_gpio40 { + brcm,pins = <40 41 42 43 44 45>; + brcm,function = <BCM2835_FSEL_ALT4>; + }; + jtag_gpio4: jtag_gpio4 { + brcm,pins = <4 5 6 12 13>; + brcm,function = <BCM2835_FSEL_ALT4>; + }; + uart1_gpio14: uart1_gpio14 { + brcm,pins = <14 15>; + brcm,function = <BCM2835_FSEL_ALT5>; + }; + uart1_ctsrts_gpio16: uart1_ctsrts_gpio16 { + brcm,pins = <16 17>; + brcm,function = <BCM2835_FSEL_ALT5>; + }; + pwm0_gpio18: pwm0_gpio18 { + brcm,pins = <18>; + brcm,function = <BCM2835_FSEL_ALT5>; + }; + pwm1_gpio19: pwm1_gpio19 { + brcm,pins = <19>; + brcm,function = <BCM2835_FSEL_ALT5>; + }; + uart1_gpio32: uart1_gpio32 { + brcm,pins = <32 33>; + brcm,function = <BCM2835_FSEL_ALT5>; + }; + uart1_ctsrts_gpio30: uart1_ctsrts_gpio30 { + brcm,pins = <30 31>; + brcm,function = <BCM2835_FSEL_ALT5>; + }; + uart1_gpio40: uart1_gpio40 { + brcm,pins = <40 41>; + brcm,function = <BCM2835_FSEL_ALT5>; + }; + uart1_ctsrts_gpio42: uart1_ctsrts_gpio42 { + brcm,pins = <42 43>; + brcm,function = <BCM2835_FSEL_ALT5>; + }; }; uart0: serial@7e201000 {
The BCM2835-ARM-Peripherals.pdf documentation specifies what the function selects do for the pins, and there are a bunch of obvious groupings to be made. With these created, we'll be able to replace bcm2835-rpi.dtsi's main "set all of these pins to alt0" with references to specific groups we want enabled. Signed-off-by: Eric Anholt <eric@anholt.net> --- arch/arm/boot/dts/bcm283x.dtsi | 170 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 170 insertions(+)