lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Wed, 23 Mar 2022 17:21:21 +0100 From: Clément Léger <clement.leger@...tlin.com> To: Philipp Zabel <p.zabel@...gutronix.de> Cc: Rob Herring <robh+dt@...nel.org>, Frank Rowand <frowand.list@...il.com>, Thomas Petazzoni <thomas.petazzoni@...tlin.com>, Alexandre Belloni <alexandre.belloni@...tlin.com>, Allan Nielsen <allan.nielsen@...rochip.com>, linux-kernel@...r.kernel.org, devicetree@...r.kernel.org Subject: Re: [PATCH 2/2] reset: add support for fwnode Le Wed, 23 Mar 2022 16:29:41 +0100, Philipp Zabel <p.zabel@...gutronix.de> a écrit : > On Mi, 2022-03-23 at 10:50 +0100, Clément Léger wrote: > [...] > > diff --git a/drivers/reset/core.c b/drivers/reset/core.c > > index 61e688882643..f014da03b7c1 100644 > > --- a/drivers/reset/core.c > > +++ b/drivers/reset/core.c > > @@ -4,6 +4,7 @@ > > * > > * Copyright 2013 Philipp Zabel, Pengutronix > > */ > > +#include <linux/acpi.h> > > #include <linux/atomic.h> > > #include <linux/device.h> > > #include <linux/err.h> > > @@ -70,26 +71,49 @@ static const char *rcdev_name(struct > > reset_controller_dev *rcdev) > > if (rcdev->of_node) > > return rcdev->of_node->full_name; > > Could the above be removed, since reset_controller_register() set > rcdev->fwnode to of_fwnode_handle(rcdev->of_node) earlier? Yes, this should work in all cases, the only difference is that fwnode_get_name() returns the basename of the of_node full_name field. This is potentially a change from what was displayed before. If you are ok with that, I'll drop these lines. [...] > > + } > > + > > + if (rcdev->of_xlate) { > > + rcdev->fwnode_xlate = fwnode_of_reset_xlate; > > It should be documented that .fwnode_xlate/.fwnode_reset_n_cells are > ignored if .of_xlate is set. Acked. [...] > > if (id) { > > - index = of_property_match_string(node, > > - "reset-names", id); > > + index = fwnode_property_match_string(fwnode, "reset-names", id); > > if (index == -EILSEQ) > > return ERR_PTR(index); > > I don't think this is good enough any more. At least -ENOMEM is added > as a possible error return code by this change. Yes indeed, errors are clearly not correctly handled anymore. At least -EILSEQ won't be triggered. > > [...] > > @@ -945,6 +989,9 @@ struct reset_control *__reset_control_get(struct device *dev, const char *id, > > if (dev->of_node) > > return __of_reset_control_get(dev->of_node, id, index, shared, > > optional, acquired); > > Could the above be removed, given that __of_reset_control_get() just > wraps __fwnode_reset_control_get(), which is called right below: Oh yes, sorry for that. It can clearly be removed. [...] > > * @of_node: corresponding device tree node as phandle target > > + * @fwnode: corresponding firmware node as reference target > > * @of_reset_n_cells: number of cells in reset line specifiers > > * @of_xlate: translation function to translate from specifier as found in the > > * device tree to id as given to the reset control ops, defaults > > - * to :c:func:`of_reset_simple_xlate`. > > + * to :c:func:`fwnode_of_reset_xlate`. > > + * @fwnode_reset_n_cells: number of cells in reset line reference specifiers > > + * @fwnode_xlate: translation function to translate from reference specifier as > > + * found in the firmware node description to id as given to the > > + * reset control ops, defaults to > > + * :c:func:`fwnode_reset_simple_xlate`. > > This should mention that .fwnode_xlate is ignored/overwritten when > .of_xlate is set. Acked. > > > regards > Philipp Regards, -- Clément Léger, Embedded Linux and Kernel engineer at Bootlin https://bootlin.com
Powered by blists - more mailing lists