| Message ID | 20210510080613.7866-1-juergh@canonical.com |
|---|---|
| State | New |
| Headers | show |
| Series | [SRU,H/raspi] UBUNTU: [Config] raspi: Set BCM_VCIO=y | expand |
On 10/05/2021 09:06, Juerg Haefliger wrote: > BugLink: https://bugs.launchpad.net/bugs/1927505 > > This driver was accidentally turned into a module during a raspberrypi > update. Change it back to built-in since the driver isn't loaded > automatically. > > Signed-off-by: Juerg Haefliger <juergh@canonical.com> > --- > debian.raspi/config/config.common.ubuntu | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/debian.raspi/config/config.common.ubuntu b/debian.raspi/config/config.common.ubuntu > index 27b26cac1f35..5a771a6dc437 100644 > --- a/debian.raspi/config/config.common.ubuntu > +++ b/debian.raspi/config/config.common.ubuntu > @@ -766,7 +766,7 @@ CONFIG_BCMGENET=y > CONFIG_BCM_KONA_USB2_PHY=m > CONFIG_BCM_NET_PHYLIB=y > CONFIG_BCM_SBA_RAID=m > -CONFIG_BCM_VCIO=m > +CONFIG_BCM_VCIO=y > CONFIG_BCM_VC_SM_CMA=m > CONFIG_BCM_VIDEOCORE=y > CONFIG_BD70528_WATCHDOG=m > Acked-by: Colin Ian King <colin.king@canonical.com>
Acked-by: Tim Gardner <tim.gardner@canonical.com> Don't you also have to drop vcio from the ABI files lest you get the always amusing build failure message "EE: Missing modules (start begging for mercy)". rtg On 5/10/21 2:06 AM, Juerg Haefliger wrote: > BugLink: https://bugs.launchpad.net/bugs/1927505 > > This driver was accidentally turned into a module during a raspberrypi > update. Change it back to built-in since the driver isn't loaded > automatically. > > Signed-off-by: Juerg Haefliger <juergh@canonical.com> > --- > debian.raspi/config/config.common.ubuntu | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/debian.raspi/config/config.common.ubuntu b/debian.raspi/config/config.common.ubuntu > index 27b26cac1f35..5a771a6dc437 100644 > --- a/debian.raspi/config/config.common.ubuntu > +++ b/debian.raspi/config/config.common.ubuntu > @@ -766,7 +766,7 @@ CONFIG_BCMGENET=y > CONFIG_BCM_KONA_USB2_PHY=m > CONFIG_BCM_NET_PHYLIB=y > CONFIG_BCM_SBA_RAID=m > -CONFIG_BCM_VCIO=m > +CONFIG_BCM_VCIO=y > CONFIG_BCM_VC_SM_CMA=m > CONFIG_BCM_VIDEOCORE=y > CONFIG_BD70528_WATCHDOG=m >
On Mon, 10 May 2021 06:15:56 -0600 Tim Gardner <tim.gardner@canonical.com> wrote: > Acked-by: Tim Gardner <tim.gardner@canonical.com> > > Don't you also have to drop vcio from the ABI files lest you get the > always amusing build failure message "EE: Missing modules (start begging > for mercy)". Yes. But if I include that change in this patch then it is dropped during cranky open. Which reminds me that I should work on a cranky open fix for that... And we should fix the ABI check to not complain about 'm' -> 'y' changes (which is also on my todo list somewhere). ...Juerg > rtg > > On 5/10/21 2:06 AM, Juerg Haefliger wrote: > > BugLink: https://bugs.launchpad.net/bugs/1927505 > > > > This driver was accidentally turned into a module during a raspberrypi > > update. Change it back to built-in since the driver isn't loaded > > automatically. > > > > Signed-off-by: Juerg Haefliger <juergh@canonical.com> > > --- > > debian.raspi/config/config.common.ubuntu | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/debian.raspi/config/config.common.ubuntu b/debian.raspi/config/config.common.ubuntu > > index 27b26cac1f35..5a771a6dc437 100644 > > --- a/debian.raspi/config/config.common.ubuntu > > +++ b/debian.raspi/config/config.common.ubuntu > > @@ -766,7 +766,7 @@ CONFIG_BCMGENET=y > > CONFIG_BCM_KONA_USB2_PHY=m > > CONFIG_BCM_NET_PHYLIB=y > > CONFIG_BCM_SBA_RAID=m > > -CONFIG_BCM_VCIO=m > > +CONFIG_BCM_VCIO=y > > CONFIG_BCM_VC_SM_CMA=m > > CONFIG_BCM_VIDEOCORE=y > > CONFIG_BD70528_WATCHDOG=m > > >
On 10.05.21 15:22, Juerg Haefliger wrote: > On Mon, 10 May 2021 06:15:56 -0600 > Tim Gardner <tim.gardner@canonical.com> wrote: > >> Acked-by: Tim Gardner <tim.gardner@canonical.com> >> >> Don't you also have to drop vcio from the ABI files lest you get the >> always amusing build failure message "EE: Missing modules (start begging >> for mercy)". > > Yes. But if I include that change in this patch then it is dropped during > cranky open. Which reminds me that I should work on a cranky open fix for > that... And we should fix the ABI check to not complain about 'm' -> 'y' > changes (which is also on my todo list somewhere). Not really needed. If the changes which drop modules from abi files are there one can just move the open commit before the patch that modifies the abi. Of course one could put such a logic into open. On the other hand I have the feeling the more we put into tools the less people will double check what they do or even know it. I know that volume more and more requires doing things quickly but making everything handled automatically maybe will make us pay a different price at other stages. -Stefan > > ...Juerg > > >> rtg >> >> On 5/10/21 2:06 AM, Juerg Haefliger wrote: >>> BugLink: https://bugs.launchpad.net/bugs/1927505 >>> >>> This driver was accidentally turned into a module during a raspberrypi >>> update. Change it back to built-in since the driver isn't loaded >>> automatically. >>> >>> Signed-off-by: Juerg Haefliger <juergh@canonical.com> >>> --- >>> debian.raspi/config/config.common.ubuntu | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/debian.raspi/config/config.common.ubuntu b/debian.raspi/config/config.common.ubuntu >>> index 27b26cac1f35..5a771a6dc437 100644 >>> --- a/debian.raspi/config/config.common.ubuntu >>> +++ b/debian.raspi/config/config.common.ubuntu >>> @@ -766,7 +766,7 @@ CONFIG_BCMGENET=y >>> CONFIG_BCM_KONA_USB2_PHY=m >>> CONFIG_BCM_NET_PHYLIB=y >>> CONFIG_BCM_SBA_RAID=m >>> -CONFIG_BCM_VCIO=m >>> +CONFIG_BCM_VCIO=y >>> CONFIG_BCM_VC_SM_CMA=m >>> CONFIG_BCM_VIDEOCORE=y >>> CONFIG_BD70528_WATCHDOG=m >>> >> > >
On Tue, 11 May 2021 08:20:24 +0200 Stefan Bader <stefan.bader@canonical.com> wrote: > On 10.05.21 15:22, Juerg Haefliger wrote: > > On Mon, 10 May 2021 06:15:56 -0600 > > Tim Gardner <tim.gardner@canonical.com> wrote: > > > >> Acked-by: Tim Gardner <tim.gardner@canonical.com> > >> > >> Don't you also have to drop vcio from the ABI files lest you get the > >> always amusing build failure message "EE: Missing modules (start begging > >> for mercy)". > > > > Yes. But if I include that change in this patch then it is dropped during > > cranky open. Which reminds me that I should work on a cranky open fix for > > that... And we should fix the ABI check to not complain about 'm' -> 'y' > > changes (which is also on my todo list somewhere). > > Not really needed. I beg to disagree. > If the changes which drop modules from abi files are there > one can just move the open commit before the patch that modifies the abi. Which is a rebase and we can't force-push the main kernels, or can we? And the cranker needs to realize that the open commit has to be moved... > Of > course one could put such a logic into open. On the other hand I have the > feeling the more we put into tools the less people will double check what they > do or even know it. I know that volume more and more requires doing things > quickly but making everything handled automatically maybe will make us pay a > different price at other stages. Disabling a module is a very explicit operation and there shouldn't be any manual fixups necessary after (or during) an open commit to please the ABI checker, IMO. We do have the modules.ignore concept, so why not simply add a modules.ignore file that lists the module that is removed and make it part of the remove-module commit? And then make cranky a little smarter to preserve modules.ignore across opens. So no need to move the open commit (which I personally find very ugly anyways). I'm not cranking kernels that often anymore but how often do you guys trip over this after closing the release when running test builds? And isn't the fixup then to drop the module from the ABI list rather than moving the open commit (and redo the whole thing)? ...Juerg > -Stefan > > > > > ...Juerg > > > > > >> rtg > >> > >> On 5/10/21 2:06 AM, Juerg Haefliger wrote: > >>> BugLink: https://bugs.launchpad.net/bugs/1927505 > >>> > >>> This driver was accidentally turned into a module during a raspberrypi > >>> update. Change it back to built-in since the driver isn't loaded > >>> automatically. > >>> > >>> Signed-off-by: Juerg Haefliger <juergh@canonical.com> > >>> --- > >>> debian.raspi/config/config.common.ubuntu | 2 +- > >>> 1 file changed, 1 insertion(+), 1 deletion(-) > >>> > >>> diff --git a/debian.raspi/config/config.common.ubuntu b/debian.raspi/config/config.common.ubuntu > >>> index 27b26cac1f35..5a771a6dc437 100644 > >>> --- a/debian.raspi/config/config.common.ubuntu > >>> +++ b/debian.raspi/config/config.common.ubuntu > >>> @@ -766,7 +766,7 @@ CONFIG_BCMGENET=y > >>> CONFIG_BCM_KONA_USB2_PHY=m > >>> CONFIG_BCM_NET_PHYLIB=y > >>> CONFIG_BCM_SBA_RAID=m > >>> -CONFIG_BCM_VCIO=m > >>> +CONFIG_BCM_VCIO=y > >>> CONFIG_BCM_VC_SM_CMA=m > >>> CONFIG_BCM_VIDEOCORE=y > >>> CONFIG_BD70528_WATCHDOG=m > >>> > >> > > > > > >
On 11.05.21 09:13, Juerg Haefliger wrote: > On Tue, 11 May 2021 08:20:24 +0200 > Stefan Bader <stefan.bader@canonical.com> wrote: > >> On 10.05.21 15:22, Juerg Haefliger wrote: >>> On Mon, 10 May 2021 06:15:56 -0600 >>> Tim Gardner <tim.gardner@canonical.com> wrote: >>> >>>> Acked-by: Tim Gardner <tim.gardner@canonical.com> >>>> >>>> Don't you also have to drop vcio from the ABI files lest you get the >>>> always amusing build failure message "EE: Missing modules (start begging >>>> for mercy)". >>> >>> Yes. But if I include that change in this patch then it is dropped during >>> cranky open. Which reminds me that I should work on a cranky open fix for >>> that... And we should fix the ABI check to not complain about 'm' -> 'y' >>> changes (which is also on my todo list somewhere). >> >> Not really needed. > > I beg to disagree. > > >> If the changes which drop modules from abi files are there >> one can just move the open commit before the patch that modifies the abi. > > Which is a rebase and we can't force-push the main kernels, or can we? And > the cranker needs to realize that the open commit has to be moved... > > >> Of >> course one could put such a logic into open. On the other hand I have the >> feeling the more we put into tools the less people will double check what they >> do or even know it. I know that volume more and more requires doing things >> quickly but making everything handled automatically maybe will make us pay a >> different price at other stages. > > Disabling a module is a very explicit operation and there shouldn't be any > manual fixups necessary after (or during) an open commit to please the ABI > checker, IMO. We do have the modules.ignore concept, so why not simply add a > modules.ignore file that lists the module that is removed and make it part > of the remove-module commit? And then make cranky a little smarter to preserve > modules.ignore across opens. So no need to move the open commit (which I > personally find very ugly anyways). > > I'm not cranking kernels that often anymore but how often do you guys trip > over this after closing the release when running test builds? And isn't the > fixup then to drop the module from the ABI list rather than moving the open > commit (and redo the whole thing)? I can only speak for myself but I try to always do a git log -- debian*/abi to see whether some non cranking change meddled with abi. No by moving the open commit before the applied drop commit, git actually changes the following patch to apply to the new files. -Stefan > > ...Juerg > > >> -Stefan >> >>> >>> ...Juerg >>> >>> >>>> rtg >>>> >>>> On 5/10/21 2:06 AM, Juerg Haefliger wrote: >>>>> BugLink: https://bugs.launchpad.net/bugs/1927505 >>>>> >>>>> This driver was accidentally turned into a module during a raspberrypi >>>>> update. Change it back to built-in since the driver isn't loaded >>>>> automatically. >>>>> >>>>> Signed-off-by: Juerg Haefliger <juergh@canonical.com> >>>>> --- >>>>> debian.raspi/config/config.common.ubuntu | 2 +- >>>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>>> >>>>> diff --git a/debian.raspi/config/config.common.ubuntu b/debian.raspi/config/config.common.ubuntu >>>>> index 27b26cac1f35..5a771a6dc437 100644 >>>>> --- a/debian.raspi/config/config.common.ubuntu >>>>> +++ b/debian.raspi/config/config.common.ubuntu >>>>> @@ -766,7 +766,7 @@ CONFIG_BCMGENET=y >>>>> CONFIG_BCM_KONA_USB2_PHY=m >>>>> CONFIG_BCM_NET_PHYLIB=y >>>>> CONFIG_BCM_SBA_RAID=m >>>>> -CONFIG_BCM_VCIO=m >>>>> +CONFIG_BCM_VCIO=y >>>>> CONFIG_BCM_VC_SM_CMA=m >>>>> CONFIG_BCM_VIDEOCORE=y >>>>> CONFIG_BD70528_WATCHDOG=m >>>>> >>>> >>> >>> >> >> >
On 5/11/21 3:26 AM, Stefan Bader wrote: > On 11.05.21 09:13, Juerg Haefliger wrote: >> On Tue, 11 May 2021 08:20:24 +0200 >> Stefan Bader <stefan.bader@canonical.com> wrote: >> >>> On 10.05.21 15:22, Juerg Haefliger wrote: >>>> On Mon, 10 May 2021 06:15:56 -0600 >>>> Tim Gardner <tim.gardner@canonical.com> wrote: >>>>> Acked-by: Tim Gardner <tim.gardner@canonical.com> >>>>> >>>>> Don't you also have to drop vcio from the ABI files lest you get the >>>>> always amusing build failure message "EE: Missing modules (start >>>>> begging >>>>> for mercy)". >>>> >>>> Yes. But if I include that change in this patch then it is dropped >>>> during >>>> cranky open. Which reminds me that I should work on a cranky open >>>> fix for >>>> that... And we should fix the ABI check to not complain about 'm' -> >>>> 'y' >>>> changes (which is also on my todo list somewhere). >>> >>> Not really needed. >> >> I beg to disagree. >> >> >>> If the changes which drop modules from abi files are there >>> one can just move the open commit before the patch that modifies the >>> abi. >> >> Which is a rebase and we can't force-push the main kernels, or can we? >> And >> the cranker needs to realize that the open commit has to be moved... >> >> >>> Of >>> course one could put such a logic into open. On the other hand I have >>> the >>> feeling the more we put into tools the less people will double check >>> what they >>> do or even know it. I know that volume more and more requires doing >>> things >>> quickly but making everything handled automatically maybe will make >>> us pay a >>> different price at other stages. >> >> Disabling a module is a very explicit operation and there shouldn't be >> any >> manual fixups necessary after (or during) an open commit to please the >> ABI >> checker, IMO. We do have the modules.ignore concept, so why not simply >> add a >> modules.ignore file that lists the module that is removed and make it >> part >> of the remove-module commit? And then make cranky a little smarter to >> preserve >> modules.ignore across opens. So no need to move the open commit (which I >> personally find very ugly anyways). >> >> I'm not cranking kernels that often anymore but how often do you guys >> trip >> over this after closing the release when running test builds? And >> isn't the >> fixup then to drop the module from the ABI list rather than moving the >> open >> commit (and redo the whole thing)? > > I can only speak for myself but I try to always do a > > git log -- debian*/abi > > to see whether some non cranking change meddled with abi. No by moving > the open commit before the applied drop commit, git actually changes the > following patch to apply to the new files. > > -Stefan Is the module check even necessary anymore ? The module check was implemented back when we still thought a stable ABI was important. Since we've abandoned that philosophy some time ago, I see no real benefit to the module check. Or rather, I see no benefit to detecting that the module list has changed and one has disappeared. rtg
On 11.05.21 16:01, Tim Gardner wrote: > > > On 5/11/21 3:26 AM, Stefan Bader wrote: >> On 11.05.21 09:13, Juerg Haefliger wrote: >>> On Tue, 11 May 2021 08:20:24 +0200 >>> Stefan Bader <stefan.bader@canonical.com> wrote: >>> >>>> On 10.05.21 15:22, Juerg Haefliger wrote: >>>>> On Mon, 10 May 2021 06:15:56 -0600 >>>>> Tim Gardner <tim.gardner@canonical.com> wrote: >>>>>> Acked-by: Tim Gardner <tim.gardner@canonical.com> >>>>>> >>>>>> Don't you also have to drop vcio from the ABI files lest you get the >>>>>> always amusing build failure message "EE: Missing modules (start begging >>>>>> for mercy)". >>>>> >>>>> Yes. But if I include that change in this patch then it is dropped during >>>>> cranky open. Which reminds me that I should work on a cranky open fix for >>>>> that... And we should fix the ABI check to not complain about 'm' -> 'y' >>>>> changes (which is also on my todo list somewhere). >>>> >>>> Not really needed. >>> >>> I beg to disagree. >>> >>> >>>> If the changes which drop modules from abi files are there >>>> one can just move the open commit before the patch that modifies the abi. >>> >>> Which is a rebase and we can't force-push the main kernels, or can we? And >>> the cranker needs to realize that the open commit has to be moved... >>> >>> >>>> Of >>>> course one could put such a logic into open. On the other hand I have the >>>> feeling the more we put into tools the less people will double check what they >>>> do or even know it. I know that volume more and more requires doing things >>>> quickly but making everything handled automatically maybe will make us pay a >>>> different price at other stages. >>> >>> Disabling a module is a very explicit operation and there shouldn't be any >>> manual fixups necessary after (or during) an open commit to please the ABI >>> checker, IMO. We do have the modules.ignore concept, so why not simply add a >>> modules.ignore file that lists the module that is removed and make it part >>> of the remove-module commit? And then make cranky a little smarter to preserve >>> modules.ignore across opens. So no need to move the open commit (which I >>> personally find very ugly anyways). >>> >>> I'm not cranking kernels that often anymore but how often do you guys trip >>> over this after closing the release when running test builds? And isn't the >>> fixup then to drop the module from the ABI list rather than moving the open >>> commit (and redo the whole thing)? >> >> I can only speak for myself but I try to always do a >> >> git log -- debian*/abi >> >> to see whether some non cranking change meddled with abi. No by moving the >> open commit before the applied drop commit, git actually changes the following >> patch to apply to the new files. >> >> -Stefan > > Is the module check even necessary anymore ? The module check was implemented > back when we still thought a stable ABI was important. Since we've abandoned > that philosophy some time ago, I see no real benefit to the module check. Or > rather, I see no benefit to detecting that the module list has changed and one > has disappeared. We occasionally seem to encounter that subject in discussion and somehow we seem to find some reason (though I cannot remember right now). Probably if the annotation enforcement were in place that would make the modules list itself redundant. At least the checking. I think for having a list, there is an alternative approach to have more info about modules (like version) and use that for dependencies or provides. But I think that thread also trailed off in the mist of lost thoughts. -Stefan > > rtg
Applied to Hirsute:master-next. thank you! -Kelsey On 2021-05-10 10:06:13 , Juerg Haefliger wrote: > BugLink: https://bugs.launchpad.net/bugs/1927505 > > This driver was accidentally turned into a module during a raspberrypi > update. Change it back to built-in since the driver isn't loaded > automatically. > > Signed-off-by: Juerg Haefliger <juergh@canonical.com> > --- > debian.raspi/config/config.common.ubuntu | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/debian.raspi/config/config.common.ubuntu b/debian.raspi/config/config.common.ubuntu > index 27b26cac1f35..5a771a6dc437 100644 > --- a/debian.raspi/config/config.common.ubuntu > +++ b/debian.raspi/config/config.common.ubuntu > @@ -766,7 +766,7 @@ CONFIG_BCMGENET=y > CONFIG_BCM_KONA_USB2_PHY=m > CONFIG_BCM_NET_PHYLIB=y > CONFIG_BCM_SBA_RAID=m > -CONFIG_BCM_VCIO=m > +CONFIG_BCM_VCIO=y > CONFIG_BCM_VC_SM_CMA=m > CONFIG_BCM_VIDEOCORE=y > CONFIG_BD70528_WATCHDOG=m > -- > 2.27.0 > > > -- > kernel-team mailing list > kernel-team@lists.ubuntu.com > https://lists.ubuntu.com/mailman/listinfo/kernel-team
diff --git a/debian.raspi/config/config.common.ubuntu b/debian.raspi/config/config.common.ubuntu index 27b26cac1f35..5a771a6dc437 100644 --- a/debian.raspi/config/config.common.ubuntu +++ b/debian.raspi/config/config.common.ubuntu @@ -766,7 +766,7 @@ CONFIG_BCMGENET=y CONFIG_BCM_KONA_USB2_PHY=m CONFIG_BCM_NET_PHYLIB=y CONFIG_BCM_SBA_RAID=m -CONFIG_BCM_VCIO=m +CONFIG_BCM_VCIO=y CONFIG_BCM_VC_SM_CMA=m CONFIG_BCM_VIDEOCORE=y CONFIG_BD70528_WATCHDOG=m
BugLink: https://bugs.launchpad.net/bugs/1927505 This driver was accidentally turned into a module during a raspberrypi update. Change it back to built-in since the driver isn't loaded automatically. Signed-off-by: Juerg Haefliger <juergh@canonical.com> --- debian.raspi/config/config.common.ubuntu | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)