| Message ID | 20231208023145.1385775-1-sergey.kambalin@auriga.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Raspberry Pi 4B machine | expand |
Hi Sergey, On 8/12/23 03:31, Sergey Kambalin wrote: > Introducing Raspberry Pi 4B model. > It contains new BCM2838 SoC, PCIE subsystem, > RNG200, Thermal sensor and Genet network controller. > > It can work with recent linux kernels 6.x.x. > Two avocado tests was added to check that. > > Unit tests has been made as read/write operations > via mailbox properties. > > Genet integration test is under development. > > Every single commit > 1) builds without errors > 2) passes regression tests > 3) passes style check* > *the only exception is bcm2838-mbox-property-test.c file > containing heavy macros usage which cause a lot of > false-positives of checkpatch.pl. > > I did my best to keep the commits less than 200 changes, > but had to make some of them a bit more in order to > keep their integrity. > > This is v2 patchset with the most of v1 remarks fixed. > I named it as 'v4' because of mistakes while attempts to send previous patchsets > Please ignore all other v1...v3 patchsets except the very first one. I really want to review your series, but I have been busy and won't have time to look at it the next 2 weeks :/ Thanks Peter for reviewing the previous version :) > Sergey Kambalin (45): > Split out common part of BCM283X classes > Split out common part of peripherals > Split out raspi machine common part > Introduce BCM2838 SoC > Add GIC-400 to BCM2838 SoC > Add BCM2838 GPIO stub > Implement BCM2838 GPIO functionality > Connect SD controller to BCM2838 GPIO > Add GPIO and SD to BCM2838 periph > Add BCM2838 checkpoint support > Introduce Raspberry PI 4 machine > Temporarily disable unimplemented rpi4b devices > Add memory region for BCM2837 RPiVid ASB > Add BCM2838 PCIE Root Complex > Add BCM2838 PCIE host > Enable BCM2838 PCIE > Add RNG200 skeleton > Add RNG200 RNG and RBG > Get rid of RNG200 timer > Implement BCM2838 thermal sensor > Add clock_isp stub > Add GENET stub > Add GENET register structs. Part 1 > Add GENET register structs. Part 2 > Add GENET register structs. Part 3 > Add GENET register structs. Part 4 > Add GENET register access macros > Implement GENET register ops > Implement GENET MDIO > Implement GENET TX path > Implement GENET RX path > Enable BCM2838 GENET controller > Connect RNG200, PCIE and GENET to GIC > Add Rpi4b boot tests > Add mailbox test stub > Add mailbox test constants > Add mailbox tests tags. Part 1 > Add mailbox tests tags. Part 2 > Add mailbox tests tags. Part 3 > Add mailbox property tests. Part 1 > Add mailbox property tests. Part 2 > Add mailbox property tests. Part 3 > Add missed BCM2835 properties > Append added properties to mailbox test > Add RPi4B to paspi4.rst
On Fri, 8 Dec 2023 at 02:32, Sergey Kambalin <serg.oker@gmail.com> wrote: > > Introducing Raspberry Pi 4B model. > It contains new BCM2838 SoC, PCIE subsystem, > RNG200, Thermal sensor and Genet network controller. > > It can work with recent linux kernels 6.x.x. > Two avocado tests was added to check that. > > Unit tests has been made as read/write operations > via mailbox properties. > > Genet integration test is under development. > > Every single commit > 1) builds without errors > 2) passes regression tests > 3) passes style check* > *the only exception is bcm2838-mbox-property-test.c file > containing heavy macros usage which cause a lot of > false-positives of checkpatch.pl. > > I did my best to keep the commits less than 200 changes, > but had to make some of them a bit more in order to > keep their integrity. > > This is v2 patchset with the most of v1 remarks fixed. > I named it as 'v4' because of mistakes while attempts to send previous patchsets > Please ignore all other v1...v3 patchsets except the very first one. Thanks for the rework and resending these patches. As you've probably seen, I've reviewed patches 1-10, but I won't have time to do more than that before I break for the holiday season now; I will get back to the rest of the series in January. -- PMM
Thank you a lot for the review Peter! May I kindly ask you to take just a brief look at the first patches of GENET? I'd like to know if I've chosen the right way to replace bitfields with QEMU REG32/FIELD32 macros. Thanks, Sergey Kambalin Software Developer, Auriga Inc.
On Tue, 19 Dec 2023 at 16:18, Kambalin, Sergey <sergey.kambalin@auriga.com> wrote: > > Thank you a lot for the review Peter! > > > May I kindly ask you to take just a brief look at the first patches of GENET? I'd like to know if I've chosen the right way to replace bitfields with QEMU REG32/FIELD32 macros. The FIELD and REG32 uses look mostly OK, but the second argument to REG32() should not be 0 each time: REG32(GENET_SYS_REV_CTRL, 0) REG32(GENET_INTRL_0, 0) etc. The idea is that that second argument is the offset in the register file of the register; then the macro defines you an A_GENET_SYS_REV_CTRL which is that value, and you can use it as a case label in the "switch (offset) {" in the read/write function. I'm a also a bit confused about the use of offsetof() in patch 27. In patch 28 the implementation of bcm2838_genet_read() and bcm2838_genet_write() use a memcpy() between a local variable and memory which I'm assuming is an index into one of these register structs, which won't do the right thing if the host is big-endian. If you need a "load/store N bytes to memory in host order", we have ldn_he_p() and stn_he_p(); also available in _le_ and _be_ flavours for load/store in specific endianness. thanks -- PMM
Thanks! I know about the 'offset' parameter, but in this particular case I use these structures as layouts only and don't 'switch' over them. So I decided to set the offsets to 0 in order to simplify the code. And extra thanks for highlighting the potential issue with memcpy()
On Tue, 19 Dec 2023 at 16:11, Peter Maydell <peter.maydell@linaro.org> wrote: > > On Fri, 8 Dec 2023 at 02:32, Sergey Kambalin <serg.oker@gmail.com> wrote: > > > > Introducing Raspberry Pi 4B model. > > It contains new BCM2838 SoC, PCIE subsystem, > > RNG200, Thermal sensor and Genet network controller. > > > > It can work with recent linux kernels 6.x.x. > > Two avocado tests was added to check that. > > > > Unit tests has been made as read/write operations > > via mailbox properties. > > > > Genet integration test is under development. > > > > Every single commit > > 1) builds without errors > > 2) passes regression tests > > 3) passes style check* > > *the only exception is bcm2838-mbox-property-test.c file > > containing heavy macros usage which cause a lot of > > false-positives of checkpatch.pl. > > > > I did my best to keep the commits less than 200 changes, > > but had to make some of them a bit more in order to > > keep their integrity. > > > > This is v2 patchset with the most of v1 remarks fixed. > > I named it as 'v4' because of mistakes while attempts to send previous patchsets > > Please ignore all other v1...v3 patchsets except the very first one. > > Thanks for the rework and resending these patches. As > you've probably seen, I've reviewed patches 1-10, but > I won't have time to do more than that before I break > for the holiday season now; I will get back to the > rest of the series in January. I'm now done with my review of this v4. I haven't left comments on every patch, but I have read through the others (including the ethernet controller ones) and I didn't see anything obviously wrong beyond what we've already discussed in this thread. I'll do a more full review for v5. thanks -- PMM