NAK: [SRU][J/allwinner-5.17][PATCH 0/4] Fix more Allwinner D1 drivers

[Emil Renner Berthing]

On Fri, 8 Jul 2022 at 15:59, Tim Gardner <tim.gardner at canonical.com> wrote:
>
> On 7/8/22 07:41, Emil Renner Berthing wrote:
> > On Fri, 8 Jul 2022 at 14:40, Tim Gardner <tim.gardner at canonical.com> wrote:
> >>
> >> On 7/8/22 04:50, Emil Renner Berthing wrote:
> >>> [Impact]
> >>>
> >>>    * The drivers for the display engine, crypto acceleration and USB
> >>>      don't probe because of missing dependencies.
> >>>
> >>> [Test Plan]
> >>>
> >>>    * Boot the kernel in linux-image-5.17.0-1002-allwinner and see
> >>>      /sys/kerne/debug/devices_deferred
> >>>
> >>> [Where problems could occur]
> >>>
> >>>    * Fixing this may introduce now bugs if the now probing drivers
> >>>      turn out to be buggy.
> >>>
> >>> Andre Przywara (1):
> >>>     phy: sun4i-usb: Rework HCI PHY (aka "pmu_unk1") handling
> >>>
> >>> Emil Renner Berthing (1):
> >>>     UBUNTU: [Config] Enable additional Allwinner D1 options
> >>>
> >>> Samuel Holland (2):
> >>>     UBUNTU: SAUCE: phy: sun4i-usb: Add D1 variant
> >>>     UBUNTU: SAUCE: PM / devfreq: Add dummy R329/D1 MBUS driver
> >>>
> >>>    debian.allwinner/config/config.common.ubuntu |  5 ++-
> >>>    drivers/devfreq/Kconfig                      |  6 +++
> >>>    drivers/devfreq/Makefile                     |  1 +
> >>>    drivers/devfreq/sun50i-r329-mbus.c           | 27 +++++++++++++
> >>>    drivers/phy/allwinner/phy-sun4i-usb.c        | 40 +++++++++++---------
> >>>    5 files changed, 60 insertions(+), 19 deletions(-)
> >>>    create mode 100644 drivers/devfreq/sun50i-r329-mbus.c
> >>>
> >>
> >> When backporting, don't forget the description of the steps you took. In
> >> this case it looks like it was simple context adjustments in patch 1/1.
> >
> > I'm sorry I don't know what you mean by this. Do you mean describe the
> > steps I took produce the error? Eg. install the allwinner kernel and
> > look at /sys/kernel/debug/devices_deferred?
> >
> > Context adjustments? Patch 1/1 is a cherry-pick from mainline and it
> > applied cleanly.
> >
>
> Patch 1/1 says '(backported from commit
> 1743dea7f06b939f67ba258bab993fa5ff6e43fb)'. This implies you had to futz
> with the patch in order to get it to apply. If it was a clean cherry
> pick, then it should say 'cherry picked from commit
> 1743dea7f06b939f67ba258bab993fa5ff6e43fb'.
>
> The description of the steps you took to apply the backport typically
> follow on the next line, e.g.,
>
> [emil - applied some grease to the muffler bearings]

Ah! Got it, thanks. Yeah, patch 1/1 should just have been
"cherry-picked from" then.

> >> Where are the annotation updates for the config changes in patch 4/4 ?
> >
> > I ran cranky updateconfigs and it didn't complain, so I guess these
> > options weren't annotated. Should I add annotations for all of them or
> > just the option added in patch 3/4?
> >
>
> Do you have 'FORMAT: 3' with "do_enforce_all=true" ?

Not in this tree unfortunately, but I'll be sure to add that when
creating the allwinner and starfive trees based on 5.19.

> The annotations are there to remind us _why_ a config option was
> changed. At the very least you'll want an annotation that enforces the
> setting along with a note referencing the LP bug. Every config should
> have an annotation, either in the master annotation file or in the
> derivative annotation file.

Right, that's of course the goal. I'll add annotations for the options
changed in this series, but do you want me to add annotations for
every other un-annotated option too?

/Emil

> > /Emil
> >
> >
> >> rtg
> >>
> >> --
> >> -----------
> >> Tim Gardner
> >> Canonical, Inc
>
>
> --
> -----------
> Tim Gardner
> Canonical, Inc