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

[Tim Gardner]

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]

>> 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" ?

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.

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


-- 
-----------
Tim Gardner
Canonical, Inc