mbox series

[SRU,J/allwinner-5.17,0/4] Fix more Allwinner D1 drivers

Message ID 20220708105033.911035-1-emil.renner.berthing@canonical.com
Headers show
Series Fix more Allwinner D1 drivers | expand

Message

Emil Renner Berthing July 8, 2022, 10:50 a.m. UTC
[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

Comments

Tim Gardner July 8, 2022, 12:40 p.m. UTC | #1
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.

Where are the annotation updates for the config changes in patch 4/4 ?

rtg
Emil Renner Berthing July 8, 2022, 1:41 p.m. UTC | #2
On Fri, 8 Jul 2022 at 14:40, Tim Gardner <tim.gardner@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.

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

/Emil


> rtg
>
> --
> -----------
> Tim Gardner
> Canonical, Inc
Tim Gardner July 8, 2022, 1:59 p.m. UTC | #3
On 7/8/22 07:41, Emil Renner Berthing wrote:
> On Fri, 8 Jul 2022 at 14:40, Tim Gardner <tim.gardner@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
Emil Renner Berthing July 8, 2022, 2:07 p.m. UTC | #4
On Fri, 8 Jul 2022 at 15:59, Tim Gardner <tim.gardner@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@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
Tim Gardner July 8, 2022, 3:04 p.m. UTC | #5
On 7/8/22 08:07, Emil Renner Berthing wrote:
> On Fri, 8 Jul 2022 at 15:59, Tim Gardner <tim.gardner@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@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?
> 

For the purposes of this patch, adding annotations for the affected 
config options is sufficient. When you transition to 'FORMAT: 3' and 
do_enforce_all you'll have to have an annotation for every config option.

rtg

> /Emil
> 
>>> /Emil
>>>
>>>
>>>> rtg
>>>>
>>>> --
>>>> -----------
>>>> Tim Gardner
>>>> Canonical, Inc
>>
>>
>> --
>> -----------
>> Tim Gardner
>> Canonical, Inc
Emil Renner Berthing July 8, 2022, 3:06 p.m. UTC | #6
On Fri, 8 Jul 2022 at 17:04, Tim Gardner <tim.gardner@canonical.com> wrote:
>
> On 7/8/22 08:07, Emil Renner Berthing wrote:
> > On Fri, 8 Jul 2022 at 15:59, Tim Gardner <tim.gardner@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@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?
> >
>
> For the purposes of this patch, adding annotations for the affected
> config options is sufficient. When you transition to 'FORMAT: 3' and
> do_enforce_all you'll have to have an annotation for every config option.

Of course. Thanks.
/Emil