MrK Posted January 23 Posted January 23 I reckon it is good idea to play with ioctl() on spi node by myself and try various even freaky configurations. will the spi-sun6i driver survive ? 0 Quote
MrK Posted January 23 Posted January 23 sorry for bad news, spi dma looked too good to be true. just like teh wifi, have a look over here: https://github.com/orangepi-xunlong/firmware/issues/1 0 Quote
ag123 Posted January 23 Author Posted January 23 Das u-boot the magic behind all the memory size detection for H616/H618 memory sizes and support ! hi all, this is a little 'off-topic' but related I've been wondering what is the change that makes it possible for the memory size on Orange Pi Zero 3, Zero 2w to be detected 'automagically'. Sometime back, Orangepi distributed quite a number of images that is matched to each different memory size 1&2 GB, 1.5GB, 4GB I originally thought that the memory size is encoded in the DTS, but that if this is true then that U-boot needs to send a different DTS / DTB template with the specific memory size to the kernel at boot. This would be bad as it means having a different configuration for every dram memory size. I had a rather difficult time chasing down where this is implemented. It turns out the *magic* is all in u-boot https://source.denx.de/u-boot/u-boot/-/commit/4b02f0120a4bb2a5d7081aef8cef6a4ca57e9db2 Quote sunxi: H616: add LPDDR4 DRAM support The H616 SoC family has support for several types of DRAM: DDR3, LPDDR3, DDR4 and LPDDR4. At the moment, the driver only supports DDR3 and LPDDR3 memory. Let's extend the driver to support the LPDDR4 memory. This type of memory widely used in device with T507(-H) SoC and new orangepi zero3 with H618. The compatibility with T507 is not yet complete, because there is difference in the phy_init array. The LPDDR4-2133 timings correspond to DRAM Rayson RS1G32LO4D2BDS-53BT found on the NOR SPI from the Orangepi Zero 3 4GB. Signed-off-by: Mikhail Kalashnikov <iuncuim@gmail.com> Tested-by: Piotr Oniszczuk <piotr.oniszczuk@gmail.com> Reviewed-by: Jernej Skrabec <jernej.skrabec@gmail.com> Acked-by: Andre Przywara <andre.przywara@arm.com> https://source.denx.de/u-boot/u-boot/-/blob/master/arch/arm/mach-sunxi/dram_sun50i_h616.c?ref_type=heads // SPDX-License-Identifier: GPL-2.0+ /* * sun50i H616 platform dram controller driver * ... There is a lot of codes in there that does the LPDDR4 / DDR3 initialization, clocks and detecting the memory size. We'd need to observe and keep u-boot up-to-date for this purpose as I'd guess most of us don't have a 1.5GB board, and it may fail if someone shows up here with a 1.5GB board complaining about issues. My thoughts are that 1.5GB boards if they are not yet supported would be added for support in the upstream u-boot as above. Hence, it helps to watch u-boot for additions and use an up-to-date uboot as the patches get updated. das uboot practically plays what is deemed as 'bios' in our 'legacy' PCs and notebook PCs for these boards https://source.denx.de/u-boot/u-boot/ part of the role of 'bios' are encoded as DTS templates, so that the kernel can configure the drivers it needs at boot 0 Quote
MrK Posted January 23 Posted January 23 okay, here is some more detailed insight into spi and dma .. and the answer is here: Quote static bool sun6i_spi_can_dma(struct spi_master *master, struct spi_device *spi, struct spi_transfer *xfer) { struct sun6i_spi *sspi = spi_master_get_devdata(master); /* * If the number of spi words to transfer is less or equal than * the fifo length we can just fill the fifo and wait for a single * irq, so don't bother setting up dma */ return xfer->len > sspi->cfg->fifo_depth; } if amount of data to transfer is less than size of spi fifo which is 64 bytes then driver falls back into cpu fill path. that's why 0xdeadbeef -> spi-pipe was false positive .. while this fails: root@orangepizero3:~# ./spi-test2.sh ./spi-test2.sh ++ spi-config -d /dev/spidev1.0 -q /dev/spidev1.0: mode=0, lsb=0, bits=8, speed=100000000, spiready=0 +++ ls -l ./spi-test2.sh +++ cut '-d ' -f5-5 ++ BS=138 ++ cat ./spi-test2.sh ++ spi-pipe -b 138 -n 1 -d /dev/spidev1.0 ++ hexdump -C SPI_IOC_MESSAGE: Connection timed out root@orangepizero3:~# dmesg | tail [ 13.387410] dwmac-sun8i 5020000.ethernet eth0: No Safety Features support found [ 13.387421] dwmac-sun8i 5020000.ethernet eth0: No MAC Management Counters available [ 13.387430] dwmac-sun8i 5020000.ethernet eth0: PTP not supported by HW [ 13.387754] dwmac-sun8i 5020000.ethernet eth0: configuring for phy/rgmii-rxid link mode [ 16.452945] dwmac-sun8i 5020000.ethernet eth0: Link is Up - 1Gbps/Full - flow control rx/tx [ 272.324653] spi_master spi1: RX DMA timeout [ 272.324685] spi_master spi1: spi1.0: timeout transferring 138 bytes@100000000Hz for 532(500)ms [ 272.324727] spidev spi1.0: SPI transfer failed: -110 [ 272.324756] spi_master spi1: failed to transfer one message from queue [ 272.324773] spi_master spi1: noqueue transfer failed Conclusion is only one: DMA support in spi-sun6i.c is broken (or damn broken) I will try to pin this ****** thing down and fix it. 0 Quote
Gunjan Gupta Posted January 23 Posted January 23 59 minutes ago, ag123 said: hi all, this is a little 'off-topic' but related I've been wondering what is the change that makes it possible for the memory size on Orange Pi Zero 3, Zero 2w to be detected 'automagically'. Sometime back, Orangepi distributed quite a number of images that is matched to each different memory size 1&2 GB, 1.5GB, 4GB I originally thought that the memory size is encoded in the DTS, but that if this is true then that U-boot needs to send a different DTS / DTB template with the specific memory size to the kernel at boot. This would be bad as it means having a different configuration for every dram memory size. I had a rather difficult time chasing down where this is implemented. It turns out the *magic* is all in u-boot I didn't knew you were looking for it. I fixed a bug in the detection routine for H6 and could have pointed you to the same. My patch didn't made into U-boot, but its used in Armbian to fix boot issues for Orange Pi 3 LTS and Jernej also uses it for CoreElec. The memory size detection IIRC happens by varying two parameters called rows and cols. The test includes writing 2 values one at the start and one at the end of memory size being tested and then its read back and compared not to be same. I am not 100% sure on how that helps in determining the size but it does work. In case of H6, I am guessing there was some synchronization issue that caused the test to not work properly so I added a barrier and some delay that solved the issue. 0 Quote
Stephen Graf Posted January 23 Posted January 23 8 minutes ago, Gunjan Gupta said: My patch didn't made into U-boot, When I was working with u-boot to test the orangepizero3 implementation there was an issue with memory size not being detected properly. The issue was resolved by raising the default memory speed parameter to the value recommended for LPDDR4. I have not seen an reported problems since then. 0 Quote
Gunjan Gupta Posted January 23 Posted January 23 2 minutes ago, Stephen Graf said: When I was working with u-boot to test the orangepizero3 I am sorry if I caused any confusion. But my patch was for H6 SoC not H618. The issue is on Orange Pi 3 LTS and not Zero3 0 Quote
pixdrift Posted January 23 Posted January 23 I'd be interested to know if anyone following this thread has a 1.5GB Zero3 for testing? I was planning to buy one for testing, but with such a small price difference between the 1.5GB and 4GB (and even less for the 2GB), I couldn't convince myself to buy it, and not sure why people wouldn't just choose 4GB if given the option? Perhaps 4GB wasn't available at the time the 1.5GB was released? Even the sales statistics on AliExpress suggest most people pay the few dollars extra for the 4GB. 0 Quote
Stephen Graf Posted January 24 Posted January 24 3 hours ago, pixdrift said: why people wouldn't just choose 4GB For hardware work I find 1GB is plenty. As Bill Gates once said "who needs more than 512KB". 0 Quote
ag123 Posted January 24 Author Posted January 24 hi about u-boot and "1.5G problem" take a look here https://gist.github.com/iuncuim and in particular this patch for u-boot https://gist.github.com/iuncuim/3d2b20f7274aa8ea69a634bc7aeb54a2 I guess that will fix the problems surrounding 1.5G boards, possibly that the memory isn't correctly detected. But with the patch, it should 'just work'. But i'm not sure if that may have some unintended 'side effects' I think iuncuim https://github.com/iuncuim is probably involved in developing openwrt, hopefully that he could submit his patch for consideration and added into the upstream u-boot, that would make support common for all the Orange Pi Zero 3 and Orange Pi Zero 2w variants. I don't have the 1.5G board, if all of us don't have that for now, we'd wait for 'complaints' oh yea, I'd guess most would be more 'comfortable' with the 1G, 2G, 4G 'whole numbers' while buying boards, the price isn't that different after all, lol ok news from over at DietPi, the complaint comes from there instead: https://github.com/MichaIng/DietPi/issues/6594#issuecomment-1907369622 Sadly, the 1.5GB doesn't 'just work'. It reports 2GB (and then stops): U-Boot SPL 2024.01-rc5-armbian (Jan 13 2024 - 14:02:32 +0000) DRAM: 2048 MiB Trying to boot from MMC1 NOTICE: BL31: v2.10.0 (debug):armbian NOTICE: BL31: Built : 14:02:12, Jan 13 2024 NOTICE: BL31: Detected Allwinner H616 SoC (1823) NOTICE: BL31: Found U-Boot DTB at 0x4a0b2e68, model: OrangePi Zero3 INFO: ARM GICv2 driver initialized INFO: Configuring SPC Controller INFO: PMIC: Probing AXP305 on RSB ERROR: RSB: set run-time address: 0x10003 INFO: Could not init RSB: -65539 INFO: BL31: Platform setup done INFO: BL31: Initializing runtime services INFO: BL31: cortex_a53: CPU workaround for erratum 855873 was applied INFO: BL31: cortex_a53: CPU workaround for erratum 1530924 was applied INFO: PSCI: Suspend is unavailable INFO: BL31: Preparing for EL3 exit to normal world INFO: Entry point address = 0x4a000000 INFO: SPSR = 0x3c9 INFO: Changed devicetree. ns16550_serial serial@5000000: pinctrl_select_state_full: uclass_get_device_by_phandle_id: err=-19 U-Boot 2024.01-rc5-armbian (Jan 13 2024 - 14:02:32 +0000) Allwinner Technology CPU: Allwinner H616 (SUN50I) Model: OrangePi Zero3 DRAM: 2 GiB (effective 8 EiB) we probably have to look at this patch https://gist.github.com/iuncuim/3d2b20f7274aa8ea69a634bc7aeb54a2 diff --git a/arch/arm/mach-sunxi/dram_sun50i_h616.c b/arch/arm/mach-sunxi/dram_sun50i_h616.c index c5c1331..9b51b7e 100644 --- a/arch/arm/mach-sunxi/dram_sun50i_h616.c +++ b/arch/arm/mach-sunxi/dram_sun50i_h616.c @@ -1352,6 +1352,9 @@ static unsigned long mctl_calc_size(const struct dram_config *config) u8 width = config->bus_full_width ? 4 : 2; /* 8 banks */ + u32 offset = (1ULL << (config->cols + config->rows + 3)) * width * config->ranks; + if (mctl_mem_matches(offset-0x10)) + return (1ULL << (config->cols + config->rows + 3)) * width * config->ranks * 3 / 4; return (1ULL << (config->cols + config->rows + 3)) * width * config->ranks; } applied to this file https://source.denx.de/u-boot/u-boot/-/blob/master/arch/arm/mach-sunxi/dram_sun50i_h616.c?ref_type=heads hold it there is also this patch https://gist.github.com/iuncuim/a7b01ecd919dfd82d90f0f0fe81b50fd From 4a542204ebd3ce90393c24a102679c9a3c7c13bc Mon Sep 17 00:00:00 2001 From: iuncuim <iuncuim@gmail.com> Date: Sat, 16 Dec 2023 19:00:16 +0300 Subject: [PATCH] Update dram_sun50i_h616.h and dram_sun50i_h616.c --- .../include/asm/arch-sunxi/dram_sun50i_h616.h | 1 + arch/arm/mach-sunxi/dram_sun50i_h616.c | 18 ++++++++++++++++-- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/arch/arm/include/asm/arch-sunxi/dram_sun50i_h616.h b/arch/arm/include/asm/arch-sunxi/dram_sun50i_h616.h index a8fdda124a..0d81077ce5 100644 --- a/arch/arm/include/asm/arch-sunxi/dram_sun50i_h616.h +++ b/arch/arm/include/asm/arch-sunxi/dram_sun50i_h616.h @@ -166,6 +166,7 @@ struct dram_config { u8 rows; u8 ranks; u8 bus_full_width; + bool dram_shrink; }; static inline int ns_to_t(int nanoseconds) diff --git a/arch/arm/mach-sunxi/dram_sun50i_h616.c b/arch/arm/mach-sunxi/dram_sun50i_h616.c index c5c1331a4c..3d8171e9ae 100644 --- a/arch/arm/mach-sunxi/dram_sun50i_h616.c +++ b/arch/arm/mach-sunxi/dram_sun50i_h616.c @@ -154,7 +154,10 @@ static void mctl_set_addrmap(const struct dram_config *config) /* Ranks */ if (ranks == 2) - mctl_ctl->addrmap[0] = rows + cols - 3; + if (config->dram_shrink) + mctl_ctl->addrmap[0] = rows + cols - 5; + else + mctl_ctl->addrmap[0] = rows + cols - 3; else mctl_ctl->addrmap[0] = 0x1F; @@ -204,7 +207,10 @@ static void mctl_set_addrmap(const struct dram_config *config) mctl_ctl->addrmap[7] = 0x0F0F; break; case 15: - mctl_ctl->addrmap[6] = (cols - 3) | ((cols - 3) << 8) | ((cols - 3) << 16) | 0x0F000000; + if (config->dram_shrink) + mctl_ctl->addrmap[6] = (cols - 3) | ((cols - 2) << 8) | ((cols - 2) << 16) | 0x0F000000; + else + mctl_ctl->addrmap[6] = (cols - 3) | ((cols - 3) << 8) | ((cols - 3) << 16) | 0x0F000000; mctl_ctl->addrmap[7] = 0x0F0F; break; case 16: @@ -1324,6 +1330,7 @@ static void mctl_auto_detect_dram_size(const struct dram_para *para, struct dram_config *config) { /* detect row address bits */ + config->dram_shrink = false; config->cols = 8; config->rows = 18; mctl_core_init(para, config); @@ -1345,6 +1352,11 @@ static void mctl_auto_detect_dram_size(const struct dram_para *para, config->bus_full_width))) break; } + u32 dram_size = (1ULL << (config->cols + config->rows + 3)) * (config->bus_full_width + 1) * 2 * config->ranks; + if (mctl_mem_matches(dram_size - 0x10)) { + config->dram_shrink = true; + mctl_core_init(para, config); + } } static unsigned long mctl_calc_size(const struct dram_config *config) @@ -1352,6 +1364,8 @@ static unsigned long mctl_calc_size(const struct dram_config *config) u8 width = config->bus_full_width ? 4 : 2; /* 8 banks */ + if (config->dram_shrink) + return (1ULL << (config->cols + config->rows + 3)) * width * config->ranks / 4 * 3; return (1ULL << (config->cols + config->rows + 3)) * width * config->ranks; } -- 2.43.0 and to rebuild u-boot from source Then merge that on an image for the tests. The thing is unless any one of us has a 1.5G board, it'd probably be difficult to verify if it after all works and that we'd probably need to test it all over again for 'side effects' e.g. if it break any other boards including other allwinner boards beyond just H616 / H618 that uses the same algorithm / routine for LPDDR4 (and 3? e.g. H616) dram setup and size detection https://source.denx.de/u-boot/u-boot/-/blob/master/arch/arm/mach-sunxi/dram_sun50i_h616.c?ref_type=heads hopefully that if this is indeed a stable solution that it can be upstreamed in uboot. ----- i reluctantly ordered a 1.5G board, I don't need it, but I'd help to test it as I'd guess 'sooner than later' someone would 'show up here' with that 1.5G board. when my 1.5G board arrive, I'd test that solution if indeed it fixes 1.5G board issue, and report it in here. --- just an update here https://forum.openwrt.org/t/can-someone-make-a-build-for-orange-pi-zero-3/168930/34?u=ag1233 the patches given above probably won't work on Orange Pi Zero 3 1.5 GB the author has replied and he is probably the same author who have written the dram memory support for Orange Pi Zero 3 that we are using currently ! https://source.denx.de/u-boot/u-boot/-/commit/4b02f0120a4bb2a5d7081aef8cef6a4ca57e9db2 Quote sunxi: H616: add LPDDR4 DRAM support The H616 SoC family has support for several types of DRAM: DDR3, LPDDR3, DDR4 and LPDDR4. At the moment, the driver only supports DDR3 and LPDDR3 memory. Let's extend the driver to support the LPDDR4 memory. This type of memory widely used in device with T507(-H) SoC and new orangepi zero3 with H618. The compatibility with T507 is not yet complete, because there is difference in the phy_init array. The LPDDR4-2133 timings correspond to DRAM Rayson RS1G32LO4D2BDS-53BT found on the NOR SPI from the Orangepi Zero 3 4GB. Signed-off-by: Mikhail Kalashnikov <iuncuim@gmail.com> <----- Tested-by: Piotr Oniszczuk <piotr.oniszczuk@gmail.com> Reviewed-by: Jernej Skrabec <jernej.skrabec@gmail.com> Acked-by: Andre Przywara <andre.przywara@arm.com> Notice that he is probably the same guru/person who provided the LPDDR4 support for H618 Orange Pi Zero 3, Zero 2w in the 1st place? Hence, the current status is best defined as that 1.5GB board is not supported (by mainline u-boot) for the time being. And that we'd continue to test pre-mature changes 'offline' and that most likely the support would be a future release of mainline u-boot when it is supported. In the mean time, the best deal for any one is to use the 1GB, 2GB or 4GB variants of Orange Pi Zero 3 and skipping the 1.5GB variant. 0 Quote
MrK Posted January 24 Posted January 24 Let's return for a moment to spi and dma topic. @pixdrift, @Gunjan Gupta, @Stephen Graf The issue was about dma irq number, the interrupt handler was not triggered thus vchans were not freed and that's why vchan_complete() tasklet was not able to do its job i.e. call registered callback, and in case of spi-sun6i, rx dma descriptor uses callback to release completion and move sun6i_spi_transfer_one() forward. when the callback was not called, the wait_for_completion_timeout(&sspi->dma_rx_done, timeout); used to return due to timeout only. I copied dma section from h6 dts to h616 and did not notice that the irq does not match dma vector number. I can say it works, spi flash is online, spi1 loop works, dma is enabled to both spi controllers. You are welcome to give a try. I attached two patches. 0001 is the same as previous, 0002 fixes dma irq number and this makes massive difference. 0002-orange-pi-zero3-dma-uses-right-interrupt-number.patch 0001-orangepi-zero-3-dts-restored-dma-instance-spi1-confi.patch 0 Quote
Gunjan Gupta Posted January 24 Posted January 24 @MrKAre the changes in sun6i_spi driver required? I see in first patch you have moved some logging statements around and added a new statement. In the second patch, you have changed from DMA_PREP_INTERRUPT to 0, I am not sure what the original value was. But does it work fine without changing the spi driver? I am asking as the same SPI driver will be in use across a lot of Allwinner SoCs and hence I will like to avoid making change to it if its not necessary. 0 Quote
MrK Posted January 24 Posted January 24 @Gunjan Gupta Honestly, these changes are not much required but the last one lets someone know if e.g. dma feature was enabled in dts properly. so maybe this extended logging has some value. regarding second patch, yes I changed the DMA_PREP_INTERRUPT flag to 0 for txdesc as it is meaningless because its (*callback) filed is not being set. it is NULL by default. Yes, the driver will work no worse without these small changes. I am going to design the support for chip select driven by hw master controller and this will be huge change to a few files. I wonder if I should keep it for myself if these relatively neutral changes are your concern. 0 Quote
Gunjan Gupta Posted January 24 Posted January 24 1 minute ago, MrK said: Honestly, these changes are not much required but the last one lets someone know if e.g. dma feature was enabled in dts properly. so maybe this extended logging has some value. I am not concerned about logging. The log statement that you added is fine. The logs that you moved above the if block that checked for defered probing I think were not supposed to be moved and will just cause unnecessary noise. But anyways, other than filling up couple of lines in dmesg, they won't do any harm. 4 minutes ago, MrK said: regarding second patch, yes I changed the DMA_PREP_INTERRUPT flag to 0 for txdesc as it is meaningless because its (*callback) filed is not being set. it is NULL by default. if its not required, lets skip this change. 6 minutes ago, MrK said: Yes, the driver will work no worse without these small changes. I am going to design the support for chip select driven by hw master controller and this will be huge change to a few files. If you have any other Allwinner SoC make sure to test them as well when making those changes to ensure there is no regression. If you don't have it, then I will test it once you will raise PR against Armbian repository. 0 Quote
SteeMan Posted January 24 Posted January 24 @MrK My question would be, are you planning on submitting your changes upstream to mainline? So any patches you end up with don't become a long term Armbian maintenance burden. 0 Quote
MrK Posted January 24 Posted January 24 @Gunjan Gupta re #1 - consider that logs are created in sun6i_spi_probe() which is called once so that claim about "noise" is weird and it is good to know what the errno was at least this conclusion comes from engineering practice. #2 - no reason to keep it. description about this flag goes like this: Quote @DMA_PREP_INTERRUPT - trigger an interrupt (callback) upon completion of * this transaction was there callback assigned to txdesc ? is there anywhere txdesc->calback = my_doing_ping_function ? why do I need to prove that white is white ? #3. I see. No I do not have anything other with Allwinner than zero 3. 0 Quote
MrK Posted January 24 Posted January 24 @Stephen Graf yes, I am going to fork mainline and raising PRs. I posted patches right now just maybe for assessment .. so you think it is wrong then ignore them. 0 Quote
Gunjan Gupta Posted January 24 Posted January 24 12 minutes ago, MrK said: consider that logs are created in sun6i_spi_probe() which is called once so that claim about "noise" is weird From https://docs.kernel.org/driver-api/driver-model/driver.html Quote Optionally, probe() may return -EPROBE_DEFER if the driver depends on resources that are not yet available (e.g., supplied by a driver that hasn't initialized yet). The driver core will put the device onto the deferred probe list and will try to call it again later. So it can be called multiple times. This is exactly what those if blocks were for i.e. to make probe be called again in the future if the condition is met 0 Quote
Gunjan Gupta Posted January 24 Posted January 24 21 minutes ago, MrK said: was there callback assigned to txdesc ? is there anywhere txdesc->calback = my_doing_ping_function ? that depends on what function calls that function. You can't say for all spi devices in the world that their drivers won't try to attach a callback 21 minutes ago, MrK said: why do I need to prove that white is white ? Not asking you to prove anything. Just saying don't change things if they are not needed to be changed. Whats wrong with that? It was a simple advice. Not sure why you are getting all fired up on it. 0 Quote
Stephen Graf Posted January 24 Posted January 24 14 minutes ago, MrK said: yes, I am going to fork mainline and raising PRs. I think this was meant for @SteeMan attention. I was following this discussion and I think the discussion should be going on upstream not at the Armbian level. 0 Quote
ag123 Posted January 25 Author Posted January 25 [U-Boot,v2.1,11/13] sunxi: add DRAM support to H6 https://patchwork.ozlabs.org/project/uboot/patch/20180722221334.3679-1-icenowy@aosc.io/ https://linux-sunxi.org/DRAM_Controller the (S)DRAM controller on H6, H616, H618 (for this even the technical reference manual cannot be found in the wild) is *undocumented*, it is incredible how far we have come. (the pioneers and all did a lot of work) I'd guess is a reason for the very limited support we've today e.g. the "1.5GB problem" the memory detection logic is practically 'a hack'. There are registers in the dram chips which no publicly known ways to access them fromm the system interrface in the SOC is documented, the memory sizes would otherwise be read from the registers and alleviate all these issues. accordingly to iuncuim https://forum.openwrt.org/t/can-someone-make-a-build-for-orange-pi-zero-3/168930/38?u=ag1233 it is quite possible the addresses wrap around in the 1.5GB LPDDR4 dram chips and the 'test' for memory there returns a false positive. This is practically 'dangerous' as the dram would be detected as 2GB which is actually lesser. this reflects what is observed in the 'complaint' earlier. 1.5GB would otherwise be detected as 1GB and that would have worked even if part of that goes to waste. 0 Quote
MrK Posted January 26 Posted January 26 On 1/24/2024 at 10:30 PM, Gunjan Gupta said: that depends on what function calls that function. You can't say for all spi devices in the world that their drivers won't try to attach a callback @Gunjan Gupta the function calling callback assigned to struct dma_async_tx_descriptor is vchan_complete() and it runs in soft irq context. Yes I can say no single spi device driver should be allowed to mess with dma data and api which is being used by the spi controller driver. Do you see a difference between spi controller driver and spi device ? example: spi-sun6i.c and for example spidev.c, spi controller may or even may not use dma api - that depends on sun6i_spi_can_dma() - which I have mentioned already once upon and any data which comes from dma should not be passed over to a module requesting a transfer to be processed by spi controller. A transfer is described by struct spi_transfer and this is the only data to be exchanged between spi device (e.g. spidev) and spi controller. finally blocking spi controller's function like *_transfer_one(), e.g. sun6i_spi_transfer_one() handles the struct spi_transfer and the transfer is completed with success or error as soon as the *_transfer_one() returns. It is blocking which means it requires kernel thread or userspace thread/process executed in kernel context by means of the the ioctl(), while vchan_complete() runs soft irq context stays for that callbacks are not allowed to sleep, locking critical sections is more advanced topic. this is 1st reason why spi device should not be aware of dma operation of spi controller driver. 2nd reason is what the purpose for spi device being notified twice about completion of transfer request is ? if you don't get it why twice then here is hint: once by return from blocking *_transfer_one() and second time by call to callback from vchan_complete() you suggested. answer is: it is not required and mindless. Overall you exposed such a lack of knowledge of kernel internals like execution contexts, threads vs tasklets, principles of modular software design: api exported, api inherited, even C you know barely - scope of static functions, e.g. static int sun6i_spi_prepare_dma(struct sun6i_spi *sspi, struct spi_transfer *tfr) and execution scope of local (placed on stack) variables, I mean this: struct dma_async_tx_descriptor *rxdesc, *txdesc; from the function above so you posted such a stupid comment that it took me long time to make a decision what should I do and I considered following options: - close the account and not live any reply - live the reply and close account because I don't know how such incompetent person can do a review of someones code who is more proficient in C and kernel internals. okay last but least, Do you think you are the person who knows what does not need to be change and what requires further development ? On 1/24/2024 at 10:30 PM, Gunjan Gupta said: Just saying don't change things if they are not needed to be changed you pretend to be "mr-i-know-everything" don't you ? if so, why did you want to incorporate patch with dma assigned wrong irq ? didn't you know right dma irq number for h616/618, did you? I didn't know and had to figure this out, that was complex process involving information that spi controller driver timeouts, source of h616 spi controller, h616 dma source, dynamic debugging, thesis that dma irq is not being fired proved with h616 manual and the dtsi. Quite complex process not to be solved by weak minds. 3rd - last, have you heard about dev_warn_ratelimited() ? you haven't likely. and indeed EPROBE_DEFER can cause *probe() to be called many times, no one knows at which frequency. the article you referred to doesn't state anything on frequency. there will be no PRs. @ag123 good luck. you have "brilliant" individuals. and I realized I waste my time over here. it is right time to use settings->delete account now. 0 Quote
Gunjan Gupta Posted January 26 Posted January 26 37 minutes ago, MrK said: you pretend to be "mr-i-know-everything" don't you ? No I don't know everything. Infact I always say that "Knowledge is like an ocean and we spend our whole life to get a drop out of it" But you seem to lack common sense of not touching things that are not needed to be touched. And it seems you get offended when someone tells you otherwise. 37 minutes ago, MrK said: if so, why did you want to incorporate patch with dma assigned wrong irq ? didn't you know right dma irq number for h616/618, did you? Because I am maintainer of only one H618 device i.e. Orange Pi Zero 3 which is quite new community supported device. I have other platinum support and standard support devices that I am maintainer of to support and that take more priority than this device. 37 minutes ago, MrK said: it is right time to use settings->delete account now. Let me know if you need help with it. 1 Quote
Stephen Graf Posted January 26 Posted January 26 1 hour ago, MrK said: because I don't know how such incompetent person can do a review of someones code who is more proficient in C and kernel internals. @MrK I think you owe @Gunjan Gupta an apology. There is no need for such language when people are volunteering their time and efforts, or ever for that matter. @MrK you are in the wrong forum. Changes such as you are proposing should be done at the Linux kernel level. @MrK I am not sure the following comments are worth posting. But if it helps you, I would recommend that you learn to work with others in a team environment. You will get more things done and with better results. If you had been working for me I would have fired you for behaving this way. It is just too disruptive. 2 Quote
Uko Posted January 27 Posted January 27 Hi everyone. I'm new to building Armbian images (I can build one, but I'm not a Linux expert who can confidently fix bugs). I'm writing in this topic because I have an Orange Pi Zero 2W, and some of you mentioned that it would be good to test some changes on another device with H618. At the same time, I'm not sure if that particular device is helpful, as (from what I can tell) it has the same components as OPi Zero 3. In any case, I will try to make things work with Zero 2W, and if you think it is important and you can direct me a bit, I'm happy to collaborate and contribute. 0 Quote
ag123 Posted January 27 Author Posted January 27 @Uko take a look at this comment it is 'stashed in the middle' a few pages earlier in this thread https://forum.armbian.com/topic/29202-orange-pi-zero-3/?do=findComment&comment=179400 0 Quote
vice Posted January 28 Posted January 28 Hi good day to all i have https://www.reichelt.com/be/en/raspberry-pi-trusted-platform-module-tpm-slb9670-rpi-shd-tpm-p253834.html SLB9670 TPM and i have orangepizero3.i couldn't get /dev/tpm* device and i'm only able to get these results.https://drive.google.com/file/d/1EccjEqNMowUwtCAkghE-196MrnVBZadj/view?usp=drive_link .Can you help me to define this tpm device or can you show a way?SLB9670 TPM device is using spi0 as default but at orangepizero3 there is spi1. if device dosen't work at armbian can you suggest an i2c tpm device for orangepizero3 armbian? Regards 0 Quote
pixdrift Posted January 28 Posted January 28 Hi @vice, It's important to mention what build, kernel, configuration for the OrangePi Zero 3 you are using. There are a lot of images available outside of Armbian that may confuse the issue, and the Orange Pi Zero 3 is also in active development so if you are on mainline Armbian build you may not always have the latest changes discussed here. Can you provide some more details about what image you're using? 0 Quote
pixdrift Posted January 28 Posted January 28 (edited) @MrK I missed your messages earlier and I can see your frustration. The challenge for me personally is my ability to understand what you're proposing in your changes as I am just not at that technical level (a weak mind ), but I wish I was! That absolutely doesn't mean your changes aren't valid (or correct), but I am concerned (as I believe others are) that Armbian may not be the place for these improvements to be added, and they should perhaps be discussed in a kernel dev mailing list etc. and then make their way into Armbian by way of kernel changes? I understand @Gunjan Gupta questions/concerns about changing things (potentially unnecessarily) as I feel Armbian don't want to end up maintaining a kernel patch set very few in the project understand Do you have the details for the mailing lists which deals with the components you're discussing? If not, perhaps someone from the forum can assist with a link and/or suggestion for how to get these patches/changes to the right place. I really value your technical knowledge and experience in this space because I think what you're suggesting will improve the capabilities of the boards we're using, I just want to make sure these contributions are discussed with people responsible for this code upstream so everyone (even outside of Armbian) can benefit, and the changes can be discussed and maintained with like minded developers. Edited January 28 by pixdrift 1 Quote
Recommended Posts
Join the conversation
You can post now and register later. If you have an account, sign in now to post with your account.
Note: Your post will require moderator approval before it will be visible.