Re: [PATCH 3/3] ramips: add RT6855A SoC Linux support patches
Hi! On Thu, Dec 24, 2020 at 12:52 AM Rafaël Carré wrote: > > TODO: the spi-mt7621 patches will break support for other targets Does that work on RT6855A at all? spi-mt7621 is a mediatek IP core. For a Ralink SoC I'm expected to see a controller supported by existing spi-ralink.c instead. The GPL code you linked in one of your patch only contains the driver for the ralink spi controller and I can't find the mt7621 driver (ralink_spi_bbu.c) at all. And please try your best to finish the TODOs before sending them for review. Or send it as RFC instead. I'm marking this patch rejected for the exact reasons you've realized yourself in those TODOs: 1. adding the old vendor pci drivers 2. breaking the spi driver for mt7621/mt7628. Regards, Chuanhong Guo ___ openwrt-devel mailing list openwrt-devel@lists.openwrt.org https://lists.openwrt.org/mailman/listinfo/openwrt-devel
Re: [PATCH 3/3] ramips: add RT6855A SoC Linux support patches
Hi! On Thu, Dec 24, 2020 at 10:12 AM Chuanhong Guo wrote: > > Hi! > > On Thu, Dec 24, 2020 at 12:52 AM Rafaël Carré wrote: > > > > TODO: the spi-mt7621 patches will break support for other targets > > Does that work on RT6855A at all? spi-mt7621 is a mediatek IP core. > For a Ralink SoC I'm expected to see a controller supported by > existing spi-ralink.c instead. > The GPL code you linked in one of your patch only contains > the driver for the ralink spi controller and I can't find the mt7621 > driver (ralink_spi_bbu.c) at all. Oops. I'm not careful enough. Found it now. -- Regards, Chuanhong Guo ___ openwrt-devel mailing list openwrt-devel@lists.openwrt.org https://lists.openwrt.org/mailman/listinfo/openwrt-devel
Re: [PATCH 3/3] ramips: add RT6855A SoC Linux support patches
On Thu, Dec 24, 2020 at 12:52 AM Rafaël Carré wrote: > diff --git > a/target/linux/ramips/patches-5.4/304-HACK-spi-mt7621-rt6855a-fix.patch > b/target/linux/ramips/patches-5.4/304-HACK-spi-mt7621-rt6855a-fix.patch > new file mode 100644 > index 00..972e969fc4 > --- /dev/null > +++ b/target/linux/ramips/patches-5.4/304-HACK-spi-mt7621-rt6855a-fix.patch > @@ -0,0 +1,35 @@ > +From cbc220c515f2b387462795b730b52dea96687885 Mon Sep 17 00:00:00 2001 > +From: =?UTF-8?q?Rafa=C3=ABl=20Carr=C3=A9?= > +Date: Wed, 23 Dec 2020 14:18:31 +0100 > +Subject: [PATCH 2/5] HACK: spi-mt7621: rt6855a fix > +MIME-Version: 1.0 > +Content-Type: text/plain; charset=UTF-8 > +Content-Transfer-Encoding: 8bit > + > +TODO: no idea what this does or if it's needed on other targets > +using this driver. This spi controller doesn't provide us with chipselect control, so the maximum transfer size is limited to the buffer size of MOREBUF mode. In order to get rid of that limit, we need to hold chipselect pins ourselves. It's accomplished in spi-mt7621.c this way: 1. set RS_SLAVE_SEL to 7 so that controller controls nonexistent CS7 for every transfer. 2. set actual chipselect pins by configuring CS polarity in SPI_CS_POLAR register. Now we can trigger the controller to do multiple transfers without deactivating chipselects in between, thus the transfer size limit is lifted for this controller. Datasheet for MT7688 is publicly available: https://labs.mediatek.com/en/download/50WkbgbH and you can find register details there. > + > +Signed-off-by: Rafaël Carré > +--- > + drivers/spi/spi-mt7621.c | 5 - > + 1 file changed, 4 insertions(+), 1 deletion(-) > + > +diff --git a/drivers/spi/spi-mt7621.c b/drivers/spi/spi-mt7621.c > +index 2c3b7a2a1ec7..58fb028ed776 100644 > +--- a/drivers/spi/spi-mt7621.c > b/drivers/spi/spi-mt7621.c > +@@ -87,7 +87,10 @@ static void mt7621_spi_set_cs(struct spi_device *spi, int > enable) > +* reliably) > +*/ > + master = mt7621_spi_read(rs, MT7621_SPI_MASTER); > +- master |= MASTER_RS_SLAVE_SEL | MASTER_MORE_BUFMODE; > ++ master |= MASTER_MORE_BUFMODE; > ++#ifndef CONFIG_SOC_RT6855A > ++ master |= MASTER_RS_SLAVE_SEL; > ++#endif > + master &= ~MASTER_FULL_DUPLEX; > + mt7621_spi_write(rs, MT7621_SPI_MASTER, master); > + > +-- > +2.27.0 > + -- Regards, Chuanhong Guo ___ openwrt-devel mailing list openwrt-devel@lists.openwrt.org https://lists.openwrt.org/mailman/listinfo/openwrt-devel
Re: [PATCH 3/3] ramips: add RT6855A SoC Linux support patches
Hi, On 12/24/20 03:12, Chuanhong Guo wrote: Hi! On Thu, Dec 24, 2020 at 12:52 AM Rafaël Carré wrote: TODO: the spi-mt7621 patches will break support for other targets And please try your best to finish the TODOs before sending them for review. Or send it as RFC instead. No problem. Should I replace [PATCH] by [RFC] in each e-mail? Or only the first e-mail? I'm marking this patch rejected for the exact reasons you've realized yourself in those TODOs: 1. adding the old vendor pci drivers Late yesterday I could modify pci-rt3883.c until wlan works. Now I "just" need to clean up my changes. 2. breaking the spi driver for mt7621/mt7628. Thanks for the explanations and the datasheet in your other e-mail. I will have a look at this too. Give me a couple weeks because there is still a lot of TODOs! Regards, Chuanhong Guo ___ openwrt-devel mailing list openwrt-devel@lists.openwrt.org https://lists.openwrt.org/mailman/listinfo/openwrt-devel
RE: [PATCH 3/3] ramips: add RT6855A SoC Linux support patches
Hi, > -Original Message- > From: openwrt-devel [mailto:openwrt-devel-boun...@lists.openwrt.org] > On Behalf Of Rafaël Carré > Sent: Donnerstag, 24. Dezember 2020 09:33 > To: Chuanhong Guo > Cc: OpenWrt Development List > Subject: Re: [PATCH 3/3] ramips: add RT6855A SoC Linux support patches > > Hi, > > On 12/24/20 03:12, Chuanhong Guo wrote: > > Hi! > > > > On Thu, Dec 24, 2020 at 12:52 AM Rafaël Carré > wrote: > >> > >> TODO: the spi-mt7621 patches will break support for other targets > > > > > And please try your best to finish the TODOs before sending them for > > review. Or send it as RFC instead. > > No problem. Should I replace [PATCH] by [RFC] in each e-mail? > Or only the first e-mail? [RFC PATCH] instead of [PATCH], on all patches. For later revisions, you then increase versions, like [RFC PATCH v2] etc. automatically with git format-patch -v2 etc. Best Adrian > > > I'm marking this patch rejected for the exact reasons you've realized > > yourself in those TODOs: > > 1. adding the old vendor pci drivers > > Late yesterday I could modify pci-rt3883.c until wlan works. > Now I "just" need to clean up my changes. > > > 2. breaking the spi driver for mt7621/mt7628. > > Thanks for the explanations and the datasheet in your other e-mail. > I will have a look at this too. > > Give me a couple weeks because there is still a lot of TODOs! > > > Regards, > > Chuanhong Guo > > > > > ___ > openwrt-devel mailing list > openwrt-devel@lists.openwrt.org > https://lists.openwrt.org/mailman/listinfo/openwrt-devel ___ openwrt-devel mailing list openwrt-devel@lists.openwrt.org https://lists.openwrt.org/mailman/listinfo/openwrt-devel
Re: [PATCH 3/3] ramips: add RT6855A SoC Linux support patches
Hi, On 12/24/20 03:39, Chuanhong Guo wrote: On Thu, Dec 24, 2020 at 12:52 AM Rafaël Carré wrote: diff --git a/target/linux/ramips/patches-5.4/304-HACK-spi-mt7621-rt6855a-fix.patch b/target/linux/ramips/patches-5.4/304-HACK-spi-mt7621-rt6855a-fix.patch new file mode 100644 index 00..972e969fc4 --- /dev/null +++ b/target/linux/ramips/patches-5.4/304-HACK-spi-mt7621-rt6855a-fix.patch @@ -0,0 +1,35 @@ +From cbc220c515f2b387462795b730b52dea96687885 Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Rafa=C3=ABl=20Carr=C3=A9?= +Date: Wed, 23 Dec 2020 14:18:31 +0100 +Subject: [PATCH 2/5] HACK: spi-mt7621: rt6855a fix +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +TODO: no idea what this does or if it's needed on other targets +using this driver. This spi controller doesn't provide us with chipselect control, so the maximum transfer size is limited to the buffer size of MOREBUF mode. In order to get rid of that limit, we need to hold chipselect pins ourselves. It's accomplished in spi-mt7621.c this way: 1. set RS_SLAVE_SEL to 7 so that controller controls nonexistent CS7 for every transfer. 2. set actual chipselect pins by configuring CS polarity in SPI_CS_POLAR register. Now we can trigger the controller to do multiple transfers without deactivating chipselects in between, thus the transfer size limit is lifted for this controller. Datasheet for MT7688 is publicly available: https://labs.mediatek.com/en/download/50WkbgbH and you can find register details there. Thanks for the explanation. I had a look at the datasheet, also at the commits log of spi-mt7621.c. This also explains why I had to use dd bs=32 to get correct MTD reads. I am still a bit confused by the difference between chipselect and slaveselect. On my target "rs_slave_sel" needs to have value "0 (default is flash)" to be able to read flash. https://en.wikipedia.org/wiki/Chip_select starts with "Chip select (CS) or slave select (SS)" so they can be synonymous. I guess in this case the rs_slave_sel does something different from toggling the chipselect pin of the (flash) SPI slave. I also tried different values of cs_polar in SPI_CS_POLAR, but they don't seem to have any effect, the flash can always be read as long as rs_slave_sel stays 0. I am not sure what else to try or where to look at. Should I try a different version of vendor driver (bbu_spiflash.c) maybe, to gather some more informations on this rt6855a SoC ? Another thing, I noticed "master->num_chipselect = 2;" in the driver, is there a reason for this limit, rather than 8 as rs_slave_sel is 3 bits? (or 7 to keep device 7 as non-existent) ? + +Signed-off-by: Rafaël Carré +--- + drivers/spi/spi-mt7621.c | 5 - + 1 file changed, 4 insertions(+), 1 deletion(-) + +diff --git a/drivers/spi/spi-mt7621.c b/drivers/spi/spi-mt7621.c +index 2c3b7a2a1ec7..58fb028ed776 100644 +--- a/drivers/spi/spi-mt7621.c b/drivers/spi/spi-mt7621.c +@@ -87,7 +87,10 @@ static void mt7621_spi_set_cs(struct spi_device *spi, int enable) +* reliably) +*/ + master = mt7621_spi_read(rs, MT7621_SPI_MASTER); +- master |= MASTER_RS_SLAVE_SEL | MASTER_MORE_BUFMODE; ++ master |= MASTER_MORE_BUFMODE; ++#ifndef CONFIG_SOC_RT6855A ++ master |= MASTER_RS_SLAVE_SEL; ++#endif + master &= ~MASTER_FULL_DUPLEX; + mt7621_spi_write(rs, MT7621_SPI_MASTER, master); + +-- +2.27.0 + ___ openwrt-devel mailing list openwrt-devel@lists.openwrt.org https://lists.openwrt.org/mailman/listinfo/openwrt-devel
Re: [PATCH 3/3] ramips: add RT6855A SoC Linux support patches
Hi! On Sun, Dec 27, 2020 at 8:49 PM Rafaël Carré wrote: > Thanks for the explanation. > > I had a look at the datasheet, also at the commits log of spi-mt7621.c. > This also explains why I had to use dd bs=32 to get correct MTD reads. > > I am still a bit confused by the difference between chipselect and > slaveselect. > > On my target "rs_slave_sel" needs to have value "0 (default is flash)" > to be able to read flash. > > https://en.wikipedia.org/wiki/Chip_select starts with "Chip select (CS) > or slave select (SS)" so they can be synonymous. I guess in this case > the rs_slave_sel does something different from toggling the chipselect > pin of the (flash) SPI slave. They are the same thing. If you have to set it to 0, that probably means... > I also tried different values of cs_polar in SPI_CS_POLAR, but they > don't seem to have any effect, the flash can always be read as long as > rs_slave_sel stays 0. ... there may not be a SPI_CS_POLAR on RT6855A. And you have to live with that transfer size limitation unfortunately. > I am not sure what else to try or where to look at. > Should I try a different version of vendor driver (bbu_spiflash.c) > maybe, to gather some more informations on this rt6855a SoC ? As you can't trick the hardware to control chipselect, there isn't much in current spi-mt7621.c for you to reuse. You need to write a new spi driver to use the controller as a spi flash controller with limited transfer capability, using spi-mem interface available since linux v4.18. > Another thing, I noticed "master->num_chipselect = 2;" in the driver, > is there a reason for this limit, rather than 8 as rs_slave_sel is 3 > bits? (or 7 to keep device 7 as non-existent) ? This controller was known to be used on mt7621/mt76x8 only. Both mt7621 and mt7628 only have 2 chipselect pins exposed. As it's unlikely to be used on any other new SoCs, it's filled with the actual available chipselects, which is 2. -- Regards, Chuanhong Guo ___ openwrt-devel mailing list openwrt-devel@lists.openwrt.org https://lists.openwrt.org/mailman/listinfo/openwrt-devel
