| Message ID | 20200222014038.180923-6-saravanak@google.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | Add fw_devlink kernel commandline option | expand |
On Fri, 21 Feb 2020 17:40:38 -0800, Saravana Kannan wrote: > With the addition of fw_devlink kernel commandline option, of_devlink is > redundant and not useful anymore. So, delete it. > > Signed-off-by: Saravana Kannan <saravanak@google.com> > --- > Documentation/admin-guide/kernel-parameters.txt | 6 ------ > drivers/of/property.c | 6 ------ > 2 files changed, 12 deletions(-) > Acked-by: Rob Herring <robh@kernel.org>
Hi Saravana, On Sat, Feb 22, 2020 at 2:41 AM Saravana Kannan <saravanak@google.com> wrote: > With the addition of fw_devlink kernel commandline option, of_devlink is > redundant and not useful anymore. So, delete it. > > Signed-off-by: Saravana Kannan <saravanak@google.com> Thanks for your patch! This is now commit e94f62b7140fa3da ("of: property: Delete of_devlink kernel commandline option") upstream. > --- a/Documentation/admin-guide/kernel-parameters.txt > +++ b/Documentation/admin-guide/kernel-parameters.txt > @@ -3299,12 +3299,6 @@ > This can be set from sysctl after boot. > See Documentation/admin-guide/sysctl/vm.rst for details. > > - of_devlink [OF, KNL] Create device links between consumer and > - supplier devices by scanning the devictree to infer the > - consumer/supplier relationships. A consumer device > - will not be probed until all the supplier devices have > - probed successfully. > - > ohci1394_dma=early [HW] enable debugging via the ohci1394 driver. > See Documentation/debugging-via-ohci1394.txt for more > info. While I agree with the thunk above... > diff --git a/drivers/of/property.c b/drivers/of/property.c > index 15fc9315f1a7..f104f15b57fb 100644 > --- a/drivers/of/property.c > +++ b/drivers/of/property.c > @@ -1299,15 +1299,9 @@ static int of_link_to_suppliers(struct device *dev, > return ret; > } > > -static bool of_devlink; > -core_param(of_devlink, of_devlink, bool, 0); > - > static int of_fwnode_add_links(const struct fwnode_handle *fwnode, > struct device *dev) > { > - if (!of_devlink) > - return 0; > - > if (unlikely(!is_of_node(fwnode))) > return 0; ... I have some reservations about removing the actual code. The "of_devlink" kernel parameter was supported in v5.5 and v5.6, so removing its support may silently break some setups. Is this likely to happen? Do we need a compatibility fallback that warns to user to update his kernel command line? Gr{oetje,eeting}s, Geert
On Tue, Mar 31, 2020 at 5:20 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > Hi Saravana, > > On Sat, Feb 22, 2020 at 2:41 AM Saravana Kannan <saravanak@google.com> wrote: > > With the addition of fw_devlink kernel commandline option, of_devlink is > > redundant and not useful anymore. So, delete it. > > > > Signed-off-by: Saravana Kannan <saravanak@google.com> > > Thanks for your patch! > > This is now commit e94f62b7140fa3da ("of: property: Delete of_devlink > kernel commandline option") upstream. > > > --- a/Documentation/admin-guide/kernel-parameters.txt > > +++ b/Documentation/admin-guide/kernel-parameters.txt > > @@ -3299,12 +3299,6 @@ > > This can be set from sysctl after boot. > > See Documentation/admin-guide/sysctl/vm.rst for details. > > > > - of_devlink [OF, KNL] Create device links between consumer and > > - supplier devices by scanning the devictree to infer the > > - consumer/supplier relationships. A consumer device > > - will not be probed until all the supplier devices have > > - probed successfully. > > - > > ohci1394_dma=early [HW] enable debugging via the ohci1394 driver. > > See Documentation/debugging-via-ohci1394.txt for more > > info. > > While I agree with the thunk above... > > > diff --git a/drivers/of/property.c b/drivers/of/property.c > > index 15fc9315f1a7..f104f15b57fb 100644 > > --- a/drivers/of/property.c > > +++ b/drivers/of/property.c > > @@ -1299,15 +1299,9 @@ static int of_link_to_suppliers(struct device *dev, > > return ret; > > } > > > > -static bool of_devlink; > > -core_param(of_devlink, of_devlink, bool, 0); > > - > > static int of_fwnode_add_links(const struct fwnode_handle *fwnode, > > struct device *dev) > > { > > - if (!of_devlink) > > - return 0; > > - > > if (unlikely(!is_of_node(fwnode))) > > return 0; Hi Geert, > ... I have some reservations about removing the actual code. > The "of_devlink" kernel parameter was supported in v5.5 and v5.6, so > removing its support may silently break some setups. > > Is this likely to happen? As much as I'd love to see people start using this, I doubt of_devlink has been significantly adopted outside of Android yet (I'm working on making that easier :)). I'd be happy to be proven wrong though :) of_devlink/fw_devlink is mainly useful when module loading is causing ordering and clean up issues. So if deletion of of_devlink breaks anything, I'd expect it to break in obvious ways and not silently. > Do we need a compatibility fallback that warns to user to update his kernel > command line? I don't think we need to, but I'm not strongly against your suggestion either. Let us know what you think. -Saravana
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt index 29985152b66d..6692b2aa6140 100644 --- a/Documentation/admin-guide/kernel-parameters.txt +++ b/Documentation/admin-guide/kernel-parameters.txt @@ -3299,12 +3299,6 @@ This can be set from sysctl after boot. See Documentation/admin-guide/sysctl/vm.rst for details. - of_devlink [OF, KNL] Create device links between consumer and - supplier devices by scanning the devictree to infer the - consumer/supplier relationships. A consumer device - will not be probed until all the supplier devices have - probed successfully. - ohci1394_dma=early [HW] enable debugging via the ohci1394 driver. See Documentation/debugging-via-ohci1394.txt for more info. diff --git a/drivers/of/property.c b/drivers/of/property.c index 15fc9315f1a7..f104f15b57fb 100644 --- a/drivers/of/property.c +++ b/drivers/of/property.c @@ -1299,15 +1299,9 @@ static int of_link_to_suppliers(struct device *dev, return ret; } -static bool of_devlink; -core_param(of_devlink, of_devlink, bool, 0); - static int of_fwnode_add_links(const struct fwnode_handle *fwnode, struct device *dev) { - if (!of_devlink) - return 0; - if (unlikely(!is_of_node(fwnode))) return 0;
With the addition of fw_devlink kernel commandline option, of_devlink is redundant and not useful anymore. So, delete it. Signed-off-by: Saravana Kannan <saravanak@google.com> --- Documentation/admin-guide/kernel-parameters.txt | 6 ------ drivers/of/property.c | 6 ------ 2 files changed, 12 deletions(-)