* [PATCH 0/2] Revert "device property: Keep secondary firmware node secondary by type"
@ 2021-01-05 9:11 Bard Liao
2021-01-05 9:11 ` [PATCH 1/2] " Bard Liao
2021-01-05 9:11 ` [PATCH 2/2] device property: add description of fwnode cases Bard Liao
0 siblings, 2 replies; 8+ messages in thread
From: Bard Liao @ 2021-01-05 9:11 UTC (permalink / raw)
To: gregkh, linux-kernel, andriy.shevchenko, linux-acpi,
heikki.krogerus, rafael
Cc: bard.liao
While the commit d5dcce0 ("device property: Keep secondary firmware
node secondary by type")
describes everything correct in its commit message the change it made does
the opposite and original commit c15e1bd ("device property: Fix the
secondary firmware node handling in set_primary_fwnode()") was fully
correct. Thus, revert the former one here and improve documentation.
Bard Liao (2):
Revert "device property: Keep secondary firmware node secondary by
type"
device property: add description of fwnode cases
drivers/base/core.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
--
2.17.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/2] Revert "device property: Keep secondary firmware node secondary by type"
2021-01-05 9:11 [PATCH 0/2] Revert "device property: Keep secondary firmware node secondary by type" Bard Liao
@ 2021-01-05 9:11 ` Bard Liao
2021-01-05 14:44 ` Andy Shevchenko
2021-01-07 14:10 ` Heikki Krogerus
2021-01-05 9:11 ` [PATCH 2/2] device property: add description of fwnode cases Bard Liao
1 sibling, 2 replies; 8+ messages in thread
From: Bard Liao @ 2021-01-05 9:11 UTC (permalink / raw)
To: gregkh, linux-kernel, andriy.shevchenko, linux-acpi,
heikki.krogerus, rafael
Cc: bard.liao
While the commit d5dcce0c414f ("device property: Keep secondary firmware
node secondary by type")
describes everything correct in its commit message the change it made does
the opposite and original commit c15e1bdda436 ("device property: Fix the
secondary firmware node handling in set_primary_fwnode()") was fully
correct. Thus, revert the former one here and improve documentation in
the next patch.
Fixes: d5dcce0c414f ("device property: Keep secondary firmware node secondary by type")
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
drivers/base/core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/base/core.c b/drivers/base/core.c
index 25e08e5f40bd..51b9545a050b 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -4433,7 +4433,7 @@ void set_primary_fwnode(struct device *dev, struct fwnode_handle *fwnode)
if (fwnode_is_primary(fn)) {
dev->fwnode = fn->secondary;
if (!(parent && fn == parent->fwnode))
- fn->secondary = ERR_PTR(-ENODEV);
+ fn->secondary = NULL;
} else {
dev->fwnode = NULL;
}
--
2.17.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/2] device property: add description of fwnode cases
2021-01-05 9:11 [PATCH 0/2] Revert "device property: Keep secondary firmware node secondary by type" Bard Liao
2021-01-05 9:11 ` [PATCH 1/2] " Bard Liao
@ 2021-01-05 9:11 ` Bard Liao
2021-01-05 14:44 ` Andy Shevchenko
2021-01-07 14:11 ` Heikki Krogerus
1 sibling, 2 replies; 8+ messages in thread
From: Bard Liao @ 2021-01-05 9:11 UTC (permalink / raw)
To: gregkh, linux-kernel, andriy.shevchenko, linux-acpi,
heikki.krogerus, rafael
Cc: bard.liao
There are only four valid fwnode cases which are
- primary --> secondary --> -ENODEV
- primary --> NULL
- secondary --> -ENODEV
- NULL
dev->fwnode should be converted between the 4 cases above no matter
how/when set_primary_fwnode() and set_secondary_fwnode() are called.
Describe it in the code so people will keep it in mind.
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
drivers/base/core.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/base/core.c b/drivers/base/core.c
index 51b9545a050b..17eb14607074 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -4414,6 +4414,12 @@ static inline bool fwnode_is_primary(struct fwnode_handle *fwnode)
*
* Set the device's firmware node pointer to @fwnode, but if a secondary
* firmware node of the device is present, preserve it.
+ *
+ * Valid fwnode cases are:
+ * - primary --> secondary --> -ENODEV
+ * - primary --> NULL
+ * - secondary --> -ENODEV
+ * - NULL
*/
void set_primary_fwnode(struct device *dev, struct fwnode_handle *fwnode)
{
@@ -4432,6 +4438,7 @@ void set_primary_fwnode(struct device *dev, struct fwnode_handle *fwnode)
} else {
if (fwnode_is_primary(fn)) {
dev->fwnode = fn->secondary;
+ /* Set fn->secondary = NULL to keep fn as a primary fwnode */
if (!(parent && fn == parent->fwnode))
fn->secondary = NULL;
} else {
--
2.17.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] Revert "device property: Keep secondary firmware node secondary by type"
2021-01-05 9:11 ` [PATCH 1/2] " Bard Liao
@ 2021-01-05 14:44 ` Andy Shevchenko
2021-01-07 14:10 ` Heikki Krogerus
1 sibling, 0 replies; 8+ messages in thread
From: Andy Shevchenko @ 2021-01-05 14:44 UTC (permalink / raw)
To: Bard Liao
Cc: gregkh, linux-kernel, linux-acpi, heikki.krogerus, rafael, bard.liao
On Tue, Jan 05, 2021 at 05:11:45PM +0800, Bard Liao wrote:
> While the commit d5dcce0c414f ("device property: Keep secondary firmware
> node secondary by type")
> describes everything correct in its commit message the change it made does
> the opposite and original commit c15e1bdda436 ("device property: Fix the
> secondary firmware node handling in set_primary_fwnode()") was fully
> correct. Thus, revert the former one here and improve documentation in
> the next patch.
Thanks for catching this!
Yes, my bad that I have misinterpreted Heikki's idea.
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Fixes: d5dcce0c414f ("device property: Keep secondary firmware node secondary by type")
> Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
> ---
> drivers/base/core.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index 25e08e5f40bd..51b9545a050b 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -4433,7 +4433,7 @@ void set_primary_fwnode(struct device *dev, struct fwnode_handle *fwnode)
> if (fwnode_is_primary(fn)) {
> dev->fwnode = fn->secondary;
> if (!(parent && fn == parent->fwnode))
> - fn->secondary = ERR_PTR(-ENODEV);
> + fn->secondary = NULL;
> } else {
> dev->fwnode = NULL;
> }
> --
> 2.17.1
>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] device property: add description of fwnode cases
2021-01-05 9:11 ` [PATCH 2/2] device property: add description of fwnode cases Bard Liao
@ 2021-01-05 14:44 ` Andy Shevchenko
2021-01-07 14:11 ` Heikki Krogerus
1 sibling, 0 replies; 8+ messages in thread
From: Andy Shevchenko @ 2021-01-05 14:44 UTC (permalink / raw)
To: Bard Liao
Cc: gregkh, linux-kernel, linux-acpi, heikki.krogerus, rafael, bard.liao
On Tue, Jan 05, 2021 at 05:11:46PM +0800, Bard Liao wrote:
> There are only four valid fwnode cases which are
> - primary --> secondary --> -ENODEV
> - primary --> NULL
> - secondary --> -ENODEV
> - NULL
>
> dev->fwnode should be converted between the 4 cases above no matter
> how/when set_primary_fwnode() and set_secondary_fwnode() are called.
> Describe it in the code so people will keep it in mind.
Thanks! It will help in the future to understand better this code.
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
> ---
> drivers/base/core.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index 51b9545a050b..17eb14607074 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -4414,6 +4414,12 @@ static inline bool fwnode_is_primary(struct fwnode_handle *fwnode)
> *
> * Set the device's firmware node pointer to @fwnode, but if a secondary
> * firmware node of the device is present, preserve it.
> + *
> + * Valid fwnode cases are:
> + * - primary --> secondary --> -ENODEV
> + * - primary --> NULL
> + * - secondary --> -ENODEV
> + * - NULL
> */
> void set_primary_fwnode(struct device *dev, struct fwnode_handle *fwnode)
> {
> @@ -4432,6 +4438,7 @@ void set_primary_fwnode(struct device *dev, struct fwnode_handle *fwnode)
> } else {
> if (fwnode_is_primary(fn)) {
> dev->fwnode = fn->secondary;
> + /* Set fn->secondary = NULL to keep fn as a primary fwnode */
> if (!(parent && fn == parent->fwnode))
> fn->secondary = NULL;
> } else {
> --
> 2.17.1
>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] Revert "device property: Keep secondary firmware node secondary by type"
2021-01-05 9:11 ` [PATCH 1/2] " Bard Liao
2021-01-05 14:44 ` Andy Shevchenko
@ 2021-01-07 14:10 ` Heikki Krogerus
1 sibling, 0 replies; 8+ messages in thread
From: Heikki Krogerus @ 2021-01-07 14:10 UTC (permalink / raw)
To: Bard Liao
Cc: gregkh, linux-kernel, andriy.shevchenko, linux-acpi, rafael, bard.liao
On Tue, Jan 05, 2021 at 05:11:45PM +0800, Bard Liao wrote:
> While the commit d5dcce0c414f ("device property: Keep secondary firmware
> node secondary by type")
> describes everything correct in its commit message the change it made does
> the opposite and original commit c15e1bdda436 ("device property: Fix the
> secondary firmware node handling in set_primary_fwnode()") was fully
> correct. Thus, revert the former one here and improve documentation in
> the next patch.
>
> Fixes: d5dcce0c414f ("device property: Keep secondary firmware node secondary by type")
> Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
FWIW:
Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> ---
> drivers/base/core.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index 25e08e5f40bd..51b9545a050b 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -4433,7 +4433,7 @@ void set_primary_fwnode(struct device *dev, struct fwnode_handle *fwnode)
> if (fwnode_is_primary(fn)) {
> dev->fwnode = fn->secondary;
> if (!(parent && fn == parent->fwnode))
> - fn->secondary = ERR_PTR(-ENODEV);
> + fn->secondary = NULL;
> } else {
> dev->fwnode = NULL;
> }
> --
> 2.17.1
--
heikki
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] device property: add description of fwnode cases
2021-01-05 9:11 ` [PATCH 2/2] device property: add description of fwnode cases Bard Liao
2021-01-05 14:44 ` Andy Shevchenko
@ 2021-01-07 14:11 ` Heikki Krogerus
2021-01-07 16:58 ` Rafael J. Wysocki
1 sibling, 1 reply; 8+ messages in thread
From: Heikki Krogerus @ 2021-01-07 14:11 UTC (permalink / raw)
To: Bard Liao
Cc: gregkh, linux-kernel, andriy.shevchenko, linux-acpi, rafael, bard.liao
On Tue, Jan 05, 2021 at 05:11:46PM +0800, Bard Liao wrote:
> There are only four valid fwnode cases which are
> - primary --> secondary --> -ENODEV
> - primary --> NULL
> - secondary --> -ENODEV
> - NULL
>
> dev->fwnode should be converted between the 4 cases above no matter
> how/when set_primary_fwnode() and set_secondary_fwnode() are called.
> Describe it in the code so people will keep it in mind.
>
> Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> ---
> drivers/base/core.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index 51b9545a050b..17eb14607074 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -4414,6 +4414,12 @@ static inline bool fwnode_is_primary(struct fwnode_handle *fwnode)
> *
> * Set the device's firmware node pointer to @fwnode, but if a secondary
> * firmware node of the device is present, preserve it.
> + *
> + * Valid fwnode cases are:
> + * - primary --> secondary --> -ENODEV
> + * - primary --> NULL
> + * - secondary --> -ENODEV
> + * - NULL
> */
> void set_primary_fwnode(struct device *dev, struct fwnode_handle *fwnode)
> {
> @@ -4432,6 +4438,7 @@ void set_primary_fwnode(struct device *dev, struct fwnode_handle *fwnode)
> } else {
> if (fwnode_is_primary(fn)) {
> dev->fwnode = fn->secondary;
> + /* Set fn->secondary = NULL to keep fn as a primary fwnode */
> if (!(parent && fn == parent->fwnode))
> fn->secondary = NULL;
> } else {
> --
> 2.17.1
thanks,
--
heikki
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] device property: add description of fwnode cases
2021-01-07 14:11 ` Heikki Krogerus
@ 2021-01-07 16:58 ` Rafael J. Wysocki
0 siblings, 0 replies; 8+ messages in thread
From: Rafael J. Wysocki @ 2021-01-07 16:58 UTC (permalink / raw)
To: Heikki Krogerus, Bard Liao
Cc: Greg Kroah-Hartman, Linux Kernel Mailing List, Andy Shevchenko,
ACPI Devel Maling List, Rafael J. Wysocki, bard.liao
On Thu, Jan 7, 2021 at 3:11 PM Heikki Krogerus
<heikki.krogerus@linux.intel.com> wrote:
>
> On Tue, Jan 05, 2021 at 05:11:46PM +0800, Bard Liao wrote:
> > There are only four valid fwnode cases which are
> > - primary --> secondary --> -ENODEV
> > - primary --> NULL
> > - secondary --> -ENODEV
> > - NULL
> >
> > dev->fwnode should be converted between the 4 cases above no matter
> > how/when set_primary_fwnode() and set_secondary_fwnode() are called.
> > Describe it in the code so people will keep it in mind.
> >
> > Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
>
> Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
>
> > ---
> > drivers/base/core.c | 7 +++++++
> > 1 file changed, 7 insertions(+)
> >
> > diff --git a/drivers/base/core.c b/drivers/base/core.c
> > index 51b9545a050b..17eb14607074 100644
> > --- a/drivers/base/core.c
> > +++ b/drivers/base/core.c
> > @@ -4414,6 +4414,12 @@ static inline bool fwnode_is_primary(struct fwnode_handle *fwnode)
> > *
> > * Set the device's firmware node pointer to @fwnode, but if a secondary
> > * firmware node of the device is present, preserve it.
> > + *
> > + * Valid fwnode cases are:
> > + * - primary --> secondary --> -ENODEV
> > + * - primary --> NULL
> > + * - secondary --> -ENODEV
> > + * - NULL
> > */
> > void set_primary_fwnode(struct device *dev, struct fwnode_handle *fwnode)
> > {
> > @@ -4432,6 +4438,7 @@ void set_primary_fwnode(struct device *dev, struct fwnode_handle *fwnode)
> > } else {
> > if (fwnode_is_primary(fn)) {
> > dev->fwnode = fn->secondary;
> > + /* Set fn->secondary = NULL to keep fn as a primary fwnode */
> > if (!(parent && fn == parent->fwnode))
> > fn->secondary = NULL;
> > } else {
> > --
Applied (with a minor edit in the new comment) along with the [1/2] as
5.11-rc material, thanks!
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-01-07 16:59 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-05 9:11 [PATCH 0/2] Revert "device property: Keep secondary firmware node secondary by type" Bard Liao
2021-01-05 9:11 ` [PATCH 1/2] " Bard Liao
2021-01-05 14:44 ` Andy Shevchenko
2021-01-07 14:10 ` Heikki Krogerus
2021-01-05 9:11 ` [PATCH 2/2] device property: add description of fwnode cases Bard Liao
2021-01-05 14:44 ` Andy Shevchenko
2021-01-07 14:11 ` Heikki Krogerus
2021-01-07 16:58 ` Rafael J. Wysocki
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.