Message ID | CY5PR12MB64550ACC0B90FDD120D21E47C6E4A@CY5PR12MB6455.namprd12.prod.outlook.com (mailing list archive) |
---|---|
State | Rejected, archived |
Headers | show |
Series | powerpc/powernv: use appropiate error code | expand |
Le 01/09/2023 à 19:19, mirimmad@outlook.com a écrit : > [Vous ne recevez pas souvent de courriers de mirimmad@outlook.com. Découvrez pourquoi ceci est important à https://aka.ms/LearnAboutSenderIdentification ] > > From: Immad Mir <mirimmad17@gmail.com> > > -1 is not a valid error code. This patch replaces it with -EPERM. Can you explain how it will work ? In scom_debug_init() rc is built by oring the value returned by scom_debug_init_one(). What will be the result when oring some valid values with -EPERM ? It was working well with -1 because when you or -1 with anything you get -1 as result. But with your change I don't think it will work. Christophe > > Signed-off-by: Immad Mir <mirimmad17@gmail.com> > --- > arch/powerpc/platforms/powernv/opal-xscom.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/arch/powerpc/platforms/powernv/opal-xscom.c b/arch/powerpc/platforms/powernv/opal-xscom.c > index 262cd6fac..ce4b089dd 100644 > --- a/arch/powerpc/platforms/powernv/opal-xscom.c > +++ b/arch/powerpc/platforms/powernv/opal-xscom.c > @@ -171,7 +171,7 @@ static int scom_debug_init_one(struct dentry *root, struct device_node *dn, > if (IS_ERR(dir)) { > kfree(ent->path.data); > kfree(ent); > - return -1; > + return -EPERM; > } > > debugfs_create_blob("devspec", 0400, dir, &ent->path); > @@ -191,7 +191,7 @@ static int scom_debug_init(void) > > root = debugfs_create_dir("scom", arch_debugfs_dir); > if (IS_ERR(root)) > - return -1; > + return -EPERM; > > rc = 0; > for_each_node_with_property(dn, "scom-controller") { > -- > 2.40.0 >
On 01/09/23 11:10 pm, Christophe Leroy wrote: > > Le 01/09/2023 à 19:19, mirimmad@outlook.com a écrit : >> [Vous ne recevez pas souvent de courriers de mirimmad@outlook.com. Découvrez pourquoi ceci est important à https://aka.ms/LearnAboutSenderIdentification ] >> >> From: Immad Mir <mirimmad17@gmail.com> >> >> -1 is not a valid error code. This patch replaces it with -EPERM. > Can you explain how it will work ? > In scom_debug_init() rc is built by oring the value returned by > scom_debug_init_one(). > What will be the result when oring some valid values with -EPERM ? > It was working well with -1 because when you or -1 with anything you get > -1 as result. But with your change I don't think it will work. But Isn't EPERM defined to be 1. Immad. > > Christophe > >> Signed-off-by: Immad Mir <mirimmad17@gmail.com> >> --- >> arch/powerpc/platforms/powernv/opal-xscom.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/arch/powerpc/platforms/powernv/opal-xscom.c b/arch/powerpc/platforms/powernv/opal-xscom.c >> index 262cd6fac..ce4b089dd 100644 >> --- a/arch/powerpc/platforms/powernv/opal-xscom.c >> +++ b/arch/powerpc/platforms/powernv/opal-xscom.c >> @@ -171,7 +171,7 @@ static int scom_debug_init_one(struct dentry *root, struct device_node *dn, >> if (IS_ERR(dir)) { >> kfree(ent->path.data); >> kfree(ent); >> - return -1; >> + return -EPERM; >> } >> >> debugfs_create_blob("devspec", 0400, dir, &ent->path); >> @@ -191,7 +191,7 @@ static int scom_debug_init(void) >> >> root = debugfs_create_dir("scom", arch_debugfs_dir); >> if (IS_ERR(root)) >> - return -1; >> + return -EPERM; >> >> rc = 0; >> for_each_node_with_property(dn, "scom-controller") { >> -- >> 2.40.0 >>
On 01/09/23 11:10 pm, Christophe Leroy wrote: > > Le 01/09/2023 à 19:19, mirimmad@outlook.com a écrit : >> [Vous ne recevez pas souvent de courriers de mirimmad@outlook.com. Découvrez pourquoi ceci est important à https://aka.ms/LearnAboutSenderIdentification ] >> >> From: Immad Mir <mirimmad17@gmail.com> >> >> -1 is not a valid error code. This patch replaces it with -EPERM. > Can you explain how it will work ? > In scom_debug_init() rc is built by oring the value returned by > scom_debug_init_one(). > What will be the result when oring some valid values with -EPERM ? > It was working well with -1 because when you or -1 with anything you get > -1 as result. But with your change I don't think it will work. if EPERM is not always necessarily equal to 1, we can put a check in scom_debug_init before returning rc. If it is less than 1 (because AFAIK or-ring with negative number results back into the same negative number) we set rc equal to -1. Immad. > > Christophe > >> Signed-off-by: Immad Mir <mirimmad17@gmail.com> >> --- >> arch/powerpc/platforms/powernv/opal-xscom.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/arch/powerpc/platforms/powernv/opal-xscom.c b/arch/powerpc/platforms/powernv/opal-xscom.c >> index 262cd6fac..ce4b089dd 100644 >> --- a/arch/powerpc/platforms/powernv/opal-xscom.c >> +++ b/arch/powerpc/platforms/powernv/opal-xscom.c >> @@ -171,7 +171,7 @@ static int scom_debug_init_one(struct dentry *root, struct device_node *dn, >> if (IS_ERR(dir)) { >> kfree(ent->path.data); >> kfree(ent); >> - return -1; >> + return -EPERM; >> } >> >> debugfs_create_blob("devspec", 0400, dir, &ent->path); >> @@ -191,7 +191,7 @@ static int scom_debug_init(void) >> >> root = debugfs_create_dir("scom", arch_debugfs_dir); >> if (IS_ERR(root)) >> - return -1; >> + return -EPERM; >> >> rc = 0; >> for_each_node_with_property(dn, "scom-controller") { >> -- >> 2.40.0 >>
Le 01/09/2023 à 20:03, Immad Mir a écrit : > > On 01/09/23 11:10 pm, Christophe Leroy wrote: >> >> Le 01/09/2023 à 19:19, mirimmad@outlook.com a écrit : >>> [Vous ne recevez pas souvent de courriers de mirimmad@outlook.com. >>> Découvrez pourquoi ceci est important à >>> https://aka.ms/LearnAboutSenderIdentification ] >>> >>> From: Immad Mir <mirimmad17@gmail.com> >>> >>> -1 is not a valid error code. This patch replaces it with -EPERM. >> Can you explain how it will work ? >> In scom_debug_init() rc is built by oring the value returned by >> scom_debug_init_one(). >> What will be the result when oring some valid values with -EPERM ? >> It was working well with -1 because when you or -1 with anything you get >> -1 as result. But with your change I don't think it will work. > > > if EPERM is not always necessarily equal to 1, we can put a check in > scom_debug_init before returning rc. If it is less than 1 (because AFAIK > or-ring with negative number results back into the same negative number) > we set rc equal to -1. > The point is that EPERM is 1 by coincidence, the intention here is not to return -EPERM but really -1, so by changing this you just make the code harded to understand and maintain. Christophe
diff --git a/arch/powerpc/platforms/powernv/opal-xscom.c b/arch/powerpc/platforms/powernv/opal-xscom.c index 262cd6fac..ce4b089dd 100644 --- a/arch/powerpc/platforms/powernv/opal-xscom.c +++ b/arch/powerpc/platforms/powernv/opal-xscom.c @@ -171,7 +171,7 @@ static int scom_debug_init_one(struct dentry *root, struct device_node *dn, if (IS_ERR(dir)) { kfree(ent->path.data); kfree(ent); - return -1; + return -EPERM; } debugfs_create_blob("devspec", 0400, dir, &ent->path); @@ -191,7 +191,7 @@ static int scom_debug_init(void) root = debugfs_create_dir("scom", arch_debugfs_dir); if (IS_ERR(root)) - return -1; + return -EPERM; rc = 0; for_each_node_with_property(dn, "scom-controller") {