Hi Tom, On Fri, 4 Feb 2022 at 08:37, Tom Rini <tr...@konsulko.com> wrote: > > On Fri, Feb 04, 2022 at 04:30:23PM +0100, Pali Rohár wrote: > > On Friday 04 February 2022 08:24:03 Simon Glass wrote: > > > Hi Pali, > > > > > > On Fri, 4 Feb 2022 at 07:13, Pali Rohár <p...@kernel.org> wrote: > > > > > > > > On Friday 04 February 2022 09:00:55 Tom Rini wrote: > > > > > On Fri, Feb 04, 2022 at 02:55:39PM +0100, Pali Rohár wrote: > > > > > > On Friday 04 February 2022 08:41:55 Tom Rini wrote: > > > > > > > On Fri, Feb 04, 2022 at 11:56:52AM +0100, Pali Rohár wrote: > > > > > > > > On Thursday 03 February 2022 15:02:02 Simon Glass wrote: > > > > > > > > > Hi, > > > > > > > > > > > > > > > > > > On Thu, 3 Feb 2022 at 14:53, Tom Rini <tr...@konsulko.com> > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > On Thu, Feb 03, 2022 at 04:45:44PM -0500, Tom Rini wrote: > > > > > > > > > > > On Thu, Feb 03, 2022 at 04:16:23PM -0500, Tom Rini wrote: > > > > > > > > > > > > On Thu, Feb 03, 2022 at 07:38:50PM +0100, Pali Rohár > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > Signed-off-by: Pali Rohár <p...@kernel.org> > > > > > > > > > > > > > --- > > > > > > > > > > > > > > > > > > > > > > > > > > I would really appreciate if U-Boot test framework > > > > > > > > > > > > > starts printing > > > > > > > > > > > > > deprecation warnings, instead of sending patches > > > > > > > > > > > > > which directly drop > > > > > > > > > > > > > support for some boards. > > > > > > > > > > > > > > > > > > > > > > > > > > There is absolutely no warning during building U-Boot > > > > > > > > > > > > > for RX-51 board > > > > > > > > > > > > > that this board has not been converted to DM_KEYBOARD > > > > > > > > > > > > > yet. > > > > > > > > > > > > > > > > > > > > > > > > Please send some patch that implements what you're > > > > > > > > > > > > wanting to see for > > > > > > > > > > > > how to make the warnings more visible. I do agree the > > > > > > > > > > > > warning for > > > > > > > > > > > > v2022.10 only showed up after the merge to -next once > > > > > > > > > > > > v2022.01 came out, > > > > > > > > > > > > but it's still a fairly long time to clean up the few > > > > > > > > > > > > unconverted > > > > > > > > > > > > boards. > > > > > > > > > > > > > > > > > > > > > > Oh, I see what's going on. Simon, the DM_KEYBOARD check > > > > > > > > > > > isn't > > > > > > > > > > > triggering for any boards. I'll look more. > > > > > > > > > > > > > > > > > > > > And here's where we're at. nokia_rx51 doesn't set > > > > > > > > > > CONFIG_KEYBOARD, so > > > > > > > > > > didn't trigger the warning about migration. Every > > > > > > > > > > platform that sets > > > > > > > > > > CONFIG_KEYBOARD is migrated. I don't know how many other > > > > > > > > > > platforms are > > > > > > > > > > in the situation nokia_rx51 is in. Yanking out the legacy > > > > > > > > > > code and > > > > > > > > > > seeing what fails to build, and going from there is > > > > > > > > > > probably the next > > > > > > > > > > option. > > > > > > > > > > > > > > > > > > Yes. > > > > > > > > > > > > > > > > > > As to your questoin, none that I know of. I sent a series to > > > > > > > > > drop > > > > > > > > > cfb_console which was how this used to work, although in fact > > > > > > > > > it > > > > > > > > > hasn't worked for a while. The problem here seems to be that > > > > > > > > > this > > > > > > > > > board was multiple migrations behind and so was not caught. > > > > > > > > > > > > > > > > It is not truth that cfb_console has not worked. This driver is > > > > > > > > / was > > > > > > > > working fine on n900 without any issues. > > > > > > > > > > > > > > Yes, it's such an old missed migration that there's no check for > > > > > > > it. > > > > > > > > > > > > > > > > We should be able to remove the migration check. > > > > > > > > > > > > > > > > > > Pali, just to explain from the other POV, I am finding it > > > > > > > > > increasingly > > > > > > > > > frustrating dealing with ad-hoc CONFIG options, old drivers, > > > > > > > > > etc. We > > > > > > > > > really need to complete some of the migrations we started 6 > > > > > > > > > years ago. > > > > > > > > > > > > > > > > > > Regards, > > > > > > > > > Simon > > > > > > > > > > > > > > > > Well, I see and understand you. But as I explained it more > > > > > > > > times, it was > > > > > > > > me who is/was waiting for review of n900 patches and I was not > > > > > > > > able to > > > > > > > > speed up review process. There was always some n900 patch in > > > > > > > > waiting > > > > > > > > state. > > > > > > > > > > > > > > > > Now when pending n900 patches were reviewed and merged, I > > > > > > > > prepared and > > > > > > > > sent another one -- this DM_KEYBOARD which removes one part of > > > > > > > > cfb_console code from n900 board code. > > > > > > > > > > > > > > Alright. So, do you see the warning about DM_SERIAL? Are you > > > > > > > going to > > > > > > > start work on that migration now, or in about a week or two when > > > > > > > I pick > > > > > > > this up, assuming no changes are requested? > > > > > > > > > > > > I do not see any warning, neither about DM_SERIAL nor any other in > > > > > > RX-51 > > > > > > test job which builds bootable image for RX-51. See CI link: > > > > > > https://dev.azure.com/u-boot/u-boot/_build/results?buildId=3580&view=logs&j=7f54f624-c8dd-54be-07eb-7f2dda6bba78&t=fc9e5b19-7cbc-51d2-a2e2-6e73893dece5 > > > > > > > > > > > > But there is CONFIG_DM_SERIAL warning for nokia_rx51 board in the > > > > > > Build the World omap job, see CI link: > > > > > > https://dev.azure.com/u-boot/u-boot/_build/results?buildId=3580&view=logs&j=36d465ef-27e5-5934-99d6-cf4495f0bf3d&t=2b422a7f-7065-531c-aa25-e0171b04f5d1 > > > > > > > > > > It's also visible when just doing "make CROSS_COMPILE=... > > > > > nokia_rx51_config all" so I guess you need to debug your script and > > > > > see > > > > > why it's not showing that? > > > > > > > > > > > After this DM_KEYBOARD patch, I'm planning to look at DM_VIDEO as > > > > > > Simon > > > > > > really wants it and then I can look at DM_SERIAL. > > > > > > > > > > OK. But, are you going to start before or after this gets merged? > > > > > > > > > > -- > > > > > Tom > > > > > > > > As that cfb code shares both video and keyboard functionality, switching > > > > to DM_VIDEO depends on working DM_KEYBOARD migration. So I can start > > > > working on DM_VIDEO changes after I would see how support for > > > > DM_KEYBOARD for n900 would look like. I do not want before because I > > > > would enter into rebase, conflicts, rework and reimplementation from > > > > zero circle (with which I have experience from past n900 u-boot > > > > patches). > > > > > > If the problem is reviews I can help with that. If it is lost patches > > > then we should be able to get them all in now-ish. Can you resend > > > everything that is outstanding. > > > > This is the only outstanding patch right now. All others were finally > > reviewed and merged. > > > > > Yes you do need to put drivers in drivers/ so please create a Kconfig > > > for your keyboard driver and put it in drivers/input > > > > But this is not easy and make it harder to develop and debug. All these > > functionality shares lot of functions, variables and locks; so it cannot > > be moved into drivers/input unless you want also watchdog or other code > > in drivers/input. Not mentioning that this is specific board code which > > is not on any other board, so I do not see any value of trying to invest > > lot of time in this. > > So yes, board/nokia/rx51/rx51.c is 751 lines and should get split in to > having watchdog under drivers/watchdog/ and video stuff under > drivers/video/ and keyboard under drivers/input/ as that's how a modern > board would be done. I probably should have pushed back on such a large > board file when things first came in as that's really always been the > case, but there used to be more bad examples than there are now. But at > the end of the day, I really want to see this board brought up to modern > APIs and it's not going to be used as the basis for a new board port, so > there's a smaller risk of something new using this as an example.
Well if you are OK with it, it is fine with me. I do wonder how this board got so out of date. E.g. even the DM_I2C support is actually just veneered onto the old API. Regards, Simon