Message ID | 20220510032558.10304-1-ychuang3@nuvoton.com (mailing list archive) |
---|---|
Headers | show |
Series | Add initial support for MA35D1 SoC | expand |
On Tue, May 10, 2022 at 5:25 AM Jacky Huang <ychuang3@nuvoton.com> wrote: > > This patch series adds initial support for Nuvoton MA35D1 SoC, > include initial dts and clock controller binding. > This looks fine in principle, but we are getting close to the merge window and should finalize this quickly to make it into v5.19. I see that you don't have a console device, as commented in the .dts patch. Normally I prefer merging platforms only when there is at least rudimentary support for booting into an initramfs with a serial console, but this is a flexible rule. As with the changelog text for the .dts file, please explain in the [PATCH 0/5] cover letter what the status is. Regarding continued maintainership, we should discuss how you plan to maintain this platform. In particular, there should be an entry in the MAINTAINERS file for the platform, either pointing to yourself, or adding it to the NPCM or WPCM450 entries if this chip is in the same family. Is this also a BMC implementation, or is it something different? Arnd
On 2022/5/10 下午 03:07, Arnd Bergmann wrote: > On Tue, May 10, 2022 at 5:25 AM Jacky Huang <ychuang3@nuvoton.com> wrote: >> This patch series adds initial support for Nuvoton MA35D1 SoC, >> include initial dts and clock controller binding. >> > This looks fine in principle, but we are getting close to the merge window and > should finalize this quickly to make it into v5.19. I see that you don't have a > console device, as commented in the .dts patch. Normally I prefer merging > platforms only when there is at least rudimentary support for booting into > an initramfs with a serial console, but this is a flexible rule. > > As with the changelog text for the .dts file, please explain in the [PATCH 0/5] > cover letter what the status is. > > Regarding continued maintainership, we should discuss how you plan to > maintain this platform. In particular, there should be an entry in the > MAINTAINERS > file for the platform, either pointing to yourself, or adding it to the NPCM or > WPCM450 entries if this chip is in the same family. Is this also a BMC > implementation, or is it something different? > > Arnd Hi Arnd, Thanks for your review. MA35D1 is target at consumer application, while NPCM is for BMC. MA35D1 is equipped with ARM Coretx-A35 dual-core with the M4 co-processor. Our team has developed Linux application on MA35D1 test chip in the last year, and the mass production version is wafer-out in last month. It will be announced soon. We have ported TF-A, U-Boot, OP-TEE, and Linux 5.4.y to MA35D1 platform, and have provided Yocto and Buildroot distribution for beta site evaluation. All the source code can be found at https://github.com/OpenNuvoton/MPU-Family, include the Linux 5.4.y porting on MA35D1 platform. Yes, we have console device driver ready. Please refer to https://github.com/OpenNuvoton/MA35D1_linux-5.4.y/blob/master/drivers/tty/serial/ma35d1_serial.c. But I think we have to fix coding style and have more review on it. Is the console driver must for the initial support submit, or can we submit it later? And thank you to remind us to create an entry in MAINTAINERS file. I will add the patch in the next version. Sincerely, Jacky Huang
On Tue, May 10, 2022 at 10:40 AM Jacky Huang <ychuang3@nuvoton.com> wrote: > On 2022/5/10 下午 03:07, Arnd Bergmann wrote: > > MA35D1 is target at consumer application, while NPCM is for BMC. > MA35D1 is equipped with ARM Coretx-A35 dual-core with the M4 co-processor. > > Our team has developed Linux application on MA35D1 test chip in the last > year, and > the mass production version is wafer-out in last month. It will be > announced soon. > > We have ported TF-A, U-Boot, OP-TEE, and Linux 5.4.y to MA35D1 platform, and > have provided Yocto and Buildroot distribution for beta site evaluation. > All the source > code can be found at https://github.com/OpenNuvoton/MPU-Family, include the > Linux 5.4.y porting on MA35D1 platform. Ok, thanks for the information, this is exactly what we need in the changelog text for the platform, and (if you send a pull request) in the tag description. > Yes, we have console device driver ready. Please refer to > https://github.com/OpenNuvoton/MA35D1_linux-5.4.y/blob/master/drivers/tty/serial/ma35d1_serial.c. > But I think we have to fix coding style and have more review on it. Is > the console driver must for the initial support submit, or can we submit it later? I would prefer to have it included, but it looks like this has never been reviewed, and I can immediately see a few things that need changes before it can get included, so I suppose we could merge the platform without it. The reason I'd like to have it included is that without any I/O devices it is obvious that the code you are sending has never been tested on the kernel version you are sending it against, and that makes it more likely that there are bugs. If the platform for some reason does not make it into v5.19, I would ask you to include the serial driver in the series so we can merge a working initial branch for v5.20. In the meantime, please post the driver for review to the linux-kernel and linux-serial lists by itself, and keep me on Cc. > And thank you to remind us to create an entry in MAINTAINERS file. I > will add the patch in the next version. Ok. Arnd
On 2022/5/10 下午 08:45, Arnd Bergmann wrote: > On Tue, May 10, 2022 at 10:40 AM Jacky Huang <ychuang3@nuvoton.com> wrote: >> On 2022/5/10 下午 03:07, Arnd Bergmann wrote: >> >> MA35D1 is target at consumer application, while NPCM is for BMC. >> MA35D1 is equipped with ARM Coretx-A35 dual-core with the M4 co-processor. >> >> Our team has developed Linux application on MA35D1 test chip in the last >> year, and >> the mass production version is wafer-out in last month. It will be >> announced soon. >> >> We have ported TF-A, U-Boot, OP-TEE, and Linux 5.4.y to MA35D1 platform, and >> have provided Yocto and Buildroot distribution for beta site evaluation. >> All the source >> code can be found at https://apc01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FOpenNuvoton%2FMPU-Family&data=05%7C01%7Cychuang3%40nuvoton.com%7Cf65d464391574dcf60af08da3282f453%7Ca3f24931d4034b4a94f17d83ac638e07%7C0%7C0%7C637877835284415849%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=b6sopMTwT8XT%2FR76qASvOtqw7zs9Kcp7lIxDw4O9%2FT8%3D&reserved=0, include the >> Linux 5.4.y porting on MA35D1 platform. > Ok, thanks for the information, this is exactly what we need in the > changelog text for the platform, and (if you send a pull request) > in the tag description. > >> Yes, we have console device driver ready. Please refer to >> https://apc01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FOpenNuvoton%2FMA35D1_linux-5.4.y%2Fblob%2Fmaster%2Fdrivers%2Ftty%2Fserial%2Fma35d1_serial.c&data=05%7C01%7Cychuang3%40nuvoton.com%7Cf65d464391574dcf60af08da3282f453%7Ca3f24931d4034b4a94f17d83ac638e07%7C0%7C0%7C637877835284415849%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=ULfLkju2X98pXn%2BeCWGrvEgRchIAlv%2FSECx%2BoJzSdWI%3D&reserved=0. >> But I think we have to fix coding style and have more review on it. Is >> the console driver must for the initial support submit, or can we submit it later? > I would prefer to have it included, but it looks like this has never been > reviewed, and I can immediately see a few things that need changes > before it can get included, so I suppose we could merge the platform > without it. > > The reason I'd like to have it included is that without any I/O devices > it is obvious that the code you are sending has never been tested > on the kernel version you are sending it against, and that makes it > more likely that there are bugs. > > If the platform for some reason does not make it into v5.19, I would > ask you to include the serial driver in the series so we can merge > a working initial branch for v5.20. > > In the meantime, please post the driver for review to the linux-kernel > and linux-serial lists by itself, and keep me on Cc. > >> And thank you to remind us to create an entry in MAINTAINERS file. I >> will add the patch in the next version. > Ok. > > Arnd Hi Anrd, Thanks for your kind help. Sure, we will have review on the serial driver and include the serial driver in the next submit. Best Regards, Jacky Huang
On 10/05/2022 09:07, Arnd Bergmann wrote: > On Tue, May 10, 2022 at 5:25 AM Jacky Huang <ychuang3@nuvoton.com> wrote: >> >> This patch series adds initial support for Nuvoton MA35D1 SoC, >> include initial dts and clock controller binding. >> > > This looks fine in principle, but we are getting close to the merge window and > should finalize this quickly to make it into v5.19. I see that you don't have a > console device, as commented in the .dts patch. Normally I prefer merging > platforms only when there is at least rudimentary support for booting into > an initramfs with a serial console, but this is a flexible rule. I disagree. It does not look fine - does not pass `make dtbs_check` even with Nuvoton bindings... Best regards, Krzysztof
On Thu, May 12, 2022 at 4:11 PM Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > > On 10/05/2022 09:07, Arnd Bergmann wrote: > > On Tue, May 10, 2022 at 5:25 AM Jacky Huang <ychuang3@nuvoton.com> wrote: > >> > >> This patch series adds initial support for Nuvoton MA35D1 SoC, > >> include initial dts and clock controller binding. > >> > > > > This looks fine in principle, but we are getting close to the merge window and > > should finalize this quickly to make it into v5.19. I see that you don't have a > > console device, as commented in the .dts patch. Normally I prefer merging > > platforms only when there is at least rudimentary support for booting into > > an initramfs with a serial console, but this is a flexible rule. > > I disagree. It does not look fine - does not pass `make dtbs_check` even > with Nuvoton bindings... Ok, thanks for taking a look. It was already late for 5.19 and missing the uart driver, so it was clear it had not seen actual runtime testing. Let's try aiming for 5.20 then. Arnd
On 2022/5/12 下午 10:35, Arnd Bergmann wrote: > On Thu, May 12, 2022 at 4:11 PM Krzysztof Kozlowski > <krzysztof.kozlowski@linaro.org> wrote: >> On 10/05/2022 09:07, Arnd Bergmann wrote: >>> On Tue, May 10, 2022 at 5:25 AM Jacky Huang <ychuang3@nuvoton.com> wrote: >>>> This patch series adds initial support for Nuvoton MA35D1 SoC, >>>> include initial dts and clock controller binding. >>>> >>> This looks fine in principle, but we are getting close to the merge window and >>> should finalize this quickly to make it into v5.19. I see that you don't have a >>> console device, as commented in the .dts patch. Normally I prefer merging >>> platforms only when there is at least rudimentary support for booting into >>> an initramfs with a serial console, but this is a flexible rule. >> I disagree. It does not look fine - does not pass `make dtbs_check` even >> with Nuvoton bindings... > Ok, thanks for taking a look. It was already late for 5.19 and missing the uart > driver, so it was clear it had not seen actual runtime testing. Let's try > aiming for 5.20 then. > > Arnd Thanks for your review and help. I will run the dtbs_check, add binding document, and include serial driver in the next version. sincerely, Jacky Huang
On 13/05/2022 08:53, Jacky Huang wrote: > > Thanks for your review and help. > I will run the dtbs_check, add binding document, and include serial > driver in the next version. Except dtbs_check you have to run dt_binding_check because this is the one which was failing at the first place. Best regards, Krzysztof
On 2022/5/13 下午 02:55, Krzysztof Kozlowski wrote: > On 13/05/2022 08:53, Jacky Huang wrote: >> Thanks for your review and help. >> I will run the dtbs_check, add binding document, and include serial >> driver in the next version. > Except dtbs_check you have to run dt_binding_check because this is the > one which was failing at the first place. > > Best regards, > Krzysztof OK, I got it. Thank you very much. Sincerely, Jacky Huang