[PATCH v2] vio: make remove callback return void
The driver core ignores the return value of struct bus_type::remove()
because there is only little that can be done. To simplify the quest to
make this function return void, let struct vio_driver::remove() return
void, too. All users already unconditionally return 0, this commit makes
it obvious that returning an error code is a bad idea.
Note there are two nominally different implementations for a vio bus:
one in arch/sparc/kernel/vio.c and the other in
arch/powerpc/platforms/pseries/vio.c. This patch only adapts the powerpc
one.
Before this patch for a device that was bound to a driver without a
remove callback vio_cmo_bus_remove(viodev) wasn't called. As the device
core still considers the device unbound after vio_bus_remove() returns
calling this unconditionally is the consistent behaviour which is
implemented here.
Reviewed-by: Tyrel Datwyler
Acked-by: Lijun Pan
Acked-by: Greg Kroah-Hartman
Signed-off-by: Uwe Kleine-König
---
Hello,
I dropped the sparc specific files (i.e. all that Michael Ellerman
didn't characterize as powerpc specific and verified that they are
indeed sparc-only).
The commit log is adapted accordingly.
Best regards
Uwe
arch/powerpc/include/asm/vio.h | 2 +-
arch/powerpc/platforms/pseries/vio.c | 7 +++
drivers/char/hw_random/pseries-rng.c | 3 +--
drivers/char/tpm/tpm_ibmvtpm.c | 4 +---
drivers/crypto/nx/nx-842-pseries.c | 4 +---
drivers/crypto/nx/nx.c | 4 +---
drivers/misc/ibmvmc.c| 4 +---
drivers/net/ethernet/ibm/ibmveth.c | 4 +---
drivers/net/ethernet/ibm/ibmvnic.c | 4 +---
drivers/scsi/ibmvscsi/ibmvfc.c | 3 +--
drivers/scsi/ibmvscsi/ibmvscsi.c | 4 +---
drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c | 4 +---
drivers/tty/hvc/hvcs.c | 3 +--
13 files changed, 15 insertions(+), 35 deletions(-)
diff --git a/arch/powerpc/include/asm/vio.h b/arch/powerpc/include/asm/vio.h
index 0cf52746531b..721c0d6715ac 100644
--- a/arch/powerpc/include/asm/vio.h
+++ b/arch/powerpc/include/asm/vio.h
@@ -113,7 +113,7 @@ struct vio_driver {
const char *name;
const struct vio_device_id *id_table;
int (*probe)(struct vio_dev *dev, const struct vio_device_id *id);
- int (*remove)(struct vio_dev *dev);
+ void (*remove)(struct vio_dev *dev);
/* A driver must have a get_desired_dma() function to
* be loaded in a CMO environment if it uses DMA.
*/
diff --git a/arch/powerpc/platforms/pseries/vio.c
b/arch/powerpc/platforms/pseries/vio.c
index b2797cfe4e2b..9cb4fc839fd5 100644
--- a/arch/powerpc/platforms/pseries/vio.c
+++ b/arch/powerpc/platforms/pseries/vio.c
@@ -1261,7 +1261,6 @@ static int vio_bus_remove(struct device *dev)
struct vio_dev *viodev = to_vio_dev(dev);
struct vio_driver *viodrv = to_vio_driver(dev->driver);
struct device *devptr;
- int ret = 1;
/*
* Hold a reference to the device after the remove function is called
@@ -1270,13 +1269,13 @@ static int vio_bus_remove(struct device *dev)
devptr = get_device(dev);
if (viodrv->remove)
- ret = viodrv->remove(viodev);
+ viodrv->remove(viodev);
- if (!ret && firmware_has_feature(FW_FEATURE_CMO))
+ if (firmware_has_feature(FW_FEATURE_CMO))
vio_cmo_bus_remove(viodev);
put_device(devptr);
- return ret;
+ return 0;
}
/**
diff --git a/drivers/char/hw_random/pseries-rng.c
b/drivers/char/hw_random/pseries-rng.c
index 8038a8a9fb58..f4949b689bd5 100644
--- a/drivers/char/hw_random/pseries-rng.c
+++ b/drivers/char/hw_random/pseries-rng.c
@@ -54,10 +54,9 @@ static int pseries_rng_probe(struct vio_dev *dev,
return hwrng_register(&pseries_rng);
}
-static int pseries_rng_remove(struct vio_dev *dev)
+static void pseries_rng_remove(struct vio_dev *dev)
{
hwrng_unregister(&pseries_rng);
- return 0;
}
static const struct vio_device_id pseries_rng_driver_ids[] = {
diff --git a/drivers/char/tpm/tpm_ibmvtpm.c b/drivers/char/tpm/tpm_ibmvtpm.c
index 994385bf37c0..903604769de9 100644
--- a/drivers/char/tpm/tpm_ibmvtpm.c
+++ b/drivers/char/tpm/tpm_ibmvtpm.c
@@ -343,7 +343,7 @@ static int ibmvtpm_crq_send_init_complete(struct
ibmvtpm_dev *ibmvtpm)
*
* Return: Always 0.
*/
-static int tpm_ibmvtpm_remove(struct vio_dev *vdev)
+static void tpm_ibmvtpm_remove(struct vio_dev *vdev)
{
struct tpm_chip *chip = dev_get_drvdata(&vdev->dev);
struct ibmvtpm_dev *ibmvtpm = dev_get_drvdata(&chip->dev);
@@ -372,8 +372,6 @@ static int tpm_ibmvtpm_remove(struct vio_dev *vdev)
kfree(ibmvtpm);
/* For tpm_ibmvtpm_get_desired_dma */
dev_set_drvdata(&vdev->dev, NULL);
-
- return 0;
}
/**
diff --git a/drivers/crypto/nx/nx-842-pseries.c
b/drivers/crypto/nx/nx-842-pseries.c
index 2de5e3672e42..cc8dd3072b8b 10
[PATCH v2] vio: make remove callback return void
The driver core ignores the return value of struct bus_type::remove()
because there is only little that can be done. To simplify the quest to
make this function return void, let struct vio_driver::remove() return
void, too. All users already unconditionally return 0, this commit makes
it obvious that returning an error code is a bad idea and makes it
obvious for future driver authors that returning an error code isn't
intended.
Note there are two nominally different implementations for a vio bus:
one in arch/sparc/kernel/vio.c and the other in
arch/powerpc/platforms/pseries/vio.c. I didn't care to check which
driver is using which of these busses (or if even some of them can be
used with both) and simply adapt all drivers and the two bus codes in
one go.
Note that for the powerpc implementation there is a semantical change:
Before this patch for a device that was bound to a driver without a
remove callback vio_cmo_bus_remove(viodev) wasn't called. As the device
core still considers the device unbound after vio_bus_remove() returns
calling this unconditionally is the consistent behaviour which is
implemented here.
Reviewed-by: Tyrel Datwyler
Acked-by: Lijun Pan
Acked-by: Greg Kroah-Hartman
Signed-off-by: Uwe Kleine-König
---
Hello,
v1 (sent with Message-Id: 20210127215010.99954-1-...@kleine-koenig.org>
had an back then unfulfilled precondition for a patch to
drivers/net/ethernet/ibm/ibmvnic.c. That patch already got into v5.11 as
5e9eff5dfa46 "ibmvnic: device remove has higher precedence over reset".
So the way is free for this patch.
Compared to v1 I rebased on a later linus/master and added acks.
Best regards
Uwe
arch/powerpc/include/asm/vio.h | 2 +-
arch/powerpc/platforms/pseries/vio.c | 7 +++
arch/sparc/include/asm/vio.h | 2 +-
arch/sparc/kernel/ds.c | 6 --
arch/sparc/kernel/vio.c | 4 ++--
drivers/block/sunvdc.c | 3 +--
drivers/char/hw_random/pseries-rng.c | 3 +--
drivers/char/tpm/tpm_ibmvtpm.c | 4 +---
drivers/crypto/nx/nx-842-pseries.c | 4 +---
drivers/crypto/nx/nx.c | 4 +---
drivers/misc/ibmvmc.c| 4 +---
drivers/net/ethernet/ibm/ibmveth.c | 4 +---
drivers/net/ethernet/ibm/ibmvnic.c | 4 +---
drivers/net/ethernet/sun/ldmvsw.c| 4 +---
drivers/net/ethernet/sun/sunvnet.c | 3 +--
drivers/scsi/ibmvscsi/ibmvfc.c | 3 +--
drivers/scsi/ibmvscsi/ibmvscsi.c | 4 +---
drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c | 4 +---
drivers/tty/hvc/hvcs.c | 3 +--
drivers/tty/vcc.c| 4 +---
20 files changed, 22 insertions(+), 54 deletions(-)
diff --git a/arch/powerpc/include/asm/vio.h b/arch/powerpc/include/asm/vio.h
index 0cf52746531b..721c0d6715ac 100644
--- a/arch/powerpc/include/asm/vio.h
+++ b/arch/powerpc/include/asm/vio.h
@@ -113,7 +113,7 @@ struct vio_driver {
const char *name;
const struct vio_device_id *id_table;
int (*probe)(struct vio_dev *dev, const struct vio_device_id *id);
- int (*remove)(struct vio_dev *dev);
+ void (*remove)(struct vio_dev *dev);
/* A driver must have a get_desired_dma() function to
* be loaded in a CMO environment if it uses DMA.
*/
diff --git a/arch/powerpc/platforms/pseries/vio.c
b/arch/powerpc/platforms/pseries/vio.c
index b2797cfe4e2b..9cb4fc839fd5 100644
--- a/arch/powerpc/platforms/pseries/vio.c
+++ b/arch/powerpc/platforms/pseries/vio.c
@@ -1261,7 +1261,6 @@ static int vio_bus_remove(struct device *dev)
struct vio_dev *viodev = to_vio_dev(dev);
struct vio_driver *viodrv = to_vio_driver(dev->driver);
struct device *devptr;
- int ret = 1;
/*
* Hold a reference to the device after the remove function is called
@@ -1270,13 +1269,13 @@ static int vio_bus_remove(struct device *dev)
devptr = get_device(dev);
if (viodrv->remove)
- ret = viodrv->remove(viodev);
+ viodrv->remove(viodev);
- if (!ret && firmware_has_feature(FW_FEATURE_CMO))
+ if (firmware_has_feature(FW_FEATURE_CMO))
vio_cmo_bus_remove(viodev);
put_device(devptr);
- return ret;
+ return 0;
}
/**
diff --git a/arch/sparc/include/asm/vio.h b/arch/sparc/include/asm/vio.h
index 059f0eb678e0..8a1a83bbb6d5 100644
--- a/arch/sparc/include/asm/vio.h
+++ b/arch/sparc/include/asm/vio.h
@@ -362,7 +362,7 @@ struct vio_driver {
struct list_headnode;
const struct vio_device_id *id_table;
int (*probe)(struct vio_dev *dev, const struct vio_device_id *id);
- int (*remove)(struct vio_dev *dev);
+ void (*remove)(struct vio_dev *dev);
void (*shutdown)(struct vio_dev *dev);
unsigned long driver_data;
struct device_driverdriver;
[PATCH] coresight: etm4x: Fix merge resolution for amba rework
This was non-trivial to get right because commits
c23bc382ef0e ("coresight: etm4x: Refactor probing routine") and
5214b563588e ("coresight: etm4x: Add support for sysreg only devices")
changed the code flow considerably. With this change the driver can be
built again.
Fixes: 0573d3fa4864 ("Merge branch 'devel-stable' of
git://git.armlinux.org.uk/~rmk/linux-arm into char-misc-next")
Signed-off-by: Uwe Kleine-König
---
On Fri, Feb 05, 2021 at 12:07:09PM +0100, Greg Kroah-Hartman wrote:
> On Fri, Feb 05, 2021 at 11:56:15AM +0100, Uwe Kleine-König wrote:
> > I didn't compile test, but I'm willing to bet your resolution is wrong.
> > You have no return statement in etm4_remove_dev() but its return type is
> > int and etm4_remove_amba() still returns int but should return void.
>
> Can you send a patch to fix this up?
Sure, here it comes. As I'm unsure if you want to squash it into the
merge or want to keep it separate I crafted a commit message. If you
prefer squashing feel free to do so.
This change corresponds to the merge resolution I suggested before.
Best regards
Uwe
drivers/hwtracing/coresight/coresight-etm4x-core.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c
b/drivers/hwtracing/coresight/coresight-etm4x-core.c
index bc55b261af23..c8ecd91e289e 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
@@ -1906,15 +1906,16 @@ static int __exit etm4_remove_dev(struct etmv4_drvdata
*drvdata)
cpus_read_unlock();
coresight_unregister(drvdata->csdev);
+
+ return 0;
}
-static int __exit etm4_remove_amba(struct amba_device *adev)
+static void __exit etm4_remove_amba(struct amba_device *adev)
{
struct etmv4_drvdata *drvdata = dev_get_drvdata(&adev->dev);
if (drvdata)
- return etm4_remove_dev(drvdata);
- return 0;
+ etm4_remove_dev(drvdata);
}
static int __exit etm4_remove_platform_dev(struct platform_device *pdev)
--
2.29.2
Re: [GIT PULL] immutable branch for amba changes targeting v5.12-rc1
On Fri, Feb 05, 2021 at 11:18:17AM +0100, Greg Kroah-Hartman wrote:
> On Fri, Feb 05, 2021 at 10:37:44AM +0100, Uwe Kleine-König wrote:
> > Hello Russell, hello Greg,
> >
> > On Thu, Feb 04, 2021 at 07:15:51PM +0100, Uwe Kleine-König wrote:
> > > On Thu, Feb 04, 2021 at 04:59:51PM +, Russell King - ARM Linux admin
> > > wrote:
> > > > On Thu, Feb 04, 2021 at 05:56:50PM +0100, Greg Kroah-Hartman wrote:
> > > > > On Thu, Feb 04, 2021 at 04:52:24PM +, Russell King - ARM Linux
> > > > > admin wrote:
> > > > > > On Tue, Feb 02, 2021 at 03:06:05PM +0100, Greg Kroah-Hartman wrote:
> > > > > > > I'm glad to take this through my char/misc tree, as that's where
> > > > > > > the
> > > > > > > other coresight changes flow through. So if no one else objects,
> > > > > > > I will
> > > > > > > do so...
> > > > > >
> > > > > > Greg, did you end up pulling this after all? If not, Uwe produced a
> > > > > > v2.
> > > > > > I haven't merged v2 yet as I don't know what you've done.
> > > > >
> > > > > I thought you merged this?
> > > >
> > > > I took v1, and put it in a branch I've promised in the past not to
> > > > rebase/rewind. Uwe is now asking for me to take a v2 or apply a patch
> > > > on top.
> > > >
> > > > The only reason to produce an "immutable" branch is if it's the basis
> > > > for some dependent work and you need that branch merged into other
> > > > people's trees... so the whole "lets produce a v2" is really odd
> > > > workflow... I'm confused about what I should do, and who has to be
> > > > informed which option I take.
> > > >
> > > > I'm rather lost here too.
> > >
> > > Sorry to have cause this confusion. After I saw that my initial tag
> > > missed to adapt a driver I wanted to make it easy for you to fix the
> > > situation.
> > > So I created a patch to fix it and created a second tag with the patch
> > > squashed in. Obviously only one of them have to be picked and I hoped
> > > you (= Russell + Greg) would agree which option to pick.
> > >
> > > My preference would be if you both pick up v2 of the tag to yield a
> > > history that is bisectable without build problems, but if Russell (who
> > > already picked up the broken tag) considers his tree immutable and so
> > > isn't willing to rebase, then picking up the patch is the way to go.
> >
> > OK, the current state is that Russell applied the patch fixing
> > drivers/mailbox/arm_mhuv2.c on top of merging my first tag.
> >
> > So the way forward now is that Greg pulls
> >
> > git://git.armlinux.org.uk/~rmk/linux-arm.git devel-stable
> >
> > which currently points to
> >
> > 860660fd829e ("ARM: 9055/1: mailbox: arm_mhuv2: make remove callback
> > return void")
> >
> > , into his tree that contains the hwtracing changes that conflict with my
> > changes. @Greg: Is this good enough, or do you require a dedicated tag
> > to pull that?
> >
> > I think these conflicting hwtracing changes are not yet in any of Greg's
> > trees (at least they are not in next).
> >
> > When I pull
> >
> > https://git.kernel.org/pub/scm/linux/kernel/git/coresight/linux.git next
> >
> > (currently pointing to 4e73ff249184 ("coresight: etm4x: Handle accesses
> > to TRCSTALLCTLR")) into 860660fd829e, I get a conflict in
> > drivers/hwtracing/coresight/coresight-etm4x-core.c as expected. My
> > resolution looks as follows:
>
> Ok, my resolution looked a bit different.
>
> Can you pull my char-misc-testing branch and verify I got this all
> pulled in correctly?
minor side-note: mentioning the repo url would have simplified that test.
I looked at
https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git
char-misc-testing
commit 0573d3fa48640f0fa6b105ff92dcb02b94d6c1ab now.
I didn't compile test, but I'm willing to bet your resolution is wrong.
You have no return statement in etm4_remove_dev() but its return type is
int and etm4_remove_amba() still returns int but should return void.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König|
Industrial Linux Solutions | https://www.pengutronix.de/ |
signature.asc
Description: PGP signature
Re: [GIT PULL] immutable branch for amba changes targeting v5.12-rc1
Hello Russell, hello Greg,
On Thu, Feb 04, 2021 at 07:15:51PM +0100, Uwe Kleine-König wrote:
> On Thu, Feb 04, 2021 at 04:59:51PM +, Russell King - ARM Linux admin
> wrote:
> > On Thu, Feb 04, 2021 at 05:56:50PM +0100, Greg Kroah-Hartman wrote:
> > > On Thu, Feb 04, 2021 at 04:52:24PM +, Russell King - ARM Linux admin
> > > wrote:
> > > > On Tue, Feb 02, 2021 at 03:06:05PM +0100, Greg Kroah-Hartman wrote:
> > > > > I'm glad to take this through my char/misc tree, as that's where the
> > > > > other coresight changes flow through. So if no one else objects, I
> > > > > will
> > > > > do so...
> > > >
> > > > Greg, did you end up pulling this after all? If not, Uwe produced a v2.
> > > > I haven't merged v2 yet as I don't know what you've done.
> > >
> > > I thought you merged this?
> >
> > I took v1, and put it in a branch I've promised in the past not to
> > rebase/rewind. Uwe is now asking for me to take a v2 or apply a patch
> > on top.
> >
> > The only reason to produce an "immutable" branch is if it's the basis
> > for some dependent work and you need that branch merged into other
> > people's trees... so the whole "lets produce a v2" is really odd
> > workflow... I'm confused about what I should do, and who has to be
> > informed which option I take.
> >
> > I'm rather lost here too.
>
> Sorry to have cause this confusion. After I saw that my initial tag
> missed to adapt a driver I wanted to make it easy for you to fix the
> situation.
> So I created a patch to fix it and created a second tag with the patch
> squashed in. Obviously only one of them have to be picked and I hoped
> you (= Russell + Greg) would agree which option to pick.
>
> My preference would be if you both pick up v2 of the tag to yield a
> history that is bisectable without build problems, but if Russell (who
> already picked up the broken tag) considers his tree immutable and so
> isn't willing to rebase, then picking up the patch is the way to go.
OK, the current state is that Russell applied the patch fixing
drivers/mailbox/arm_mhuv2.c on top of merging my first tag.
So the way forward now is that Greg pulls
git://git.armlinux.org.uk/~rmk/linux-arm.git devel-stable
which currently points to
860660fd829e ("ARM: 9055/1: mailbox: arm_mhuv2: make remove callback
return void")
, into his tree that contains the hwtracing changes that conflict with my
changes. @Greg: Is this good enough, or do you require a dedicated tag
to pull that?
I think these conflicting hwtracing changes are not yet in any of Greg's
trees (at least they are not in next).
When I pull
https://git.kernel.org/pub/scm/linux/kernel/git/coresight/linux.git next
(currently pointing to 4e73ff249184 ("coresight: etm4x: Handle accesses
to TRCSTALLCTLR")) into 860660fd829e, I get a conflict in
drivers/hwtracing/coresight/coresight-etm4x-core.c as expected. My
resolution looks as follows:
diff --cc drivers/hwtracing/coresight/coresight-etm4x-core.c
index 82787cba537d,5017d33ba4f5..
--- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
@@@ -1703,6 -1906,28 +1906,27 @@@ static int __exit etm4_remove_dev(struc
cpus_read_unlock();
coresight_unregister(drvdata->csdev);
+
+ return 0;
+ }
+
-static int __exit etm4_remove_amba(struct amba_device *adev)
++static void __exit etm4_remove_amba(struct amba_device *adev)
+ {
+ struct etmv4_drvdata *drvdata = dev_get_drvdata(&adev->dev);
+
+ if (drvdata)
- return etm4_remove_dev(drvdata);
- return 0;
++ etm4_remove_dev(drvdata);
+ }
+
+ static int __exit etm4_remove_platform_dev(struct platform_device *pdev)
+ {
+ int ret = 0;
+ struct etmv4_drvdata *drvdata = dev_get_drvdata(&pdev->dev);
+
+ if (drvdata)
+ ret = etm4_remove_dev(drvdata);
+ pm_runtime_disable(&pdev->dev);
+ return ret;
}
static const struct amba_id etm4_ids[] = {
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König|
Industrial Linux Solutions | https://www.pengutronix.de/ |
signature.asc
Description: PGP signature
Re: [GIT PULL] immutable branch for amba changes targeting v5.12-rc1
On Thu, Feb 04, 2021 at 04:59:51PM +, Russell King - ARM Linux admin wrote: > On Thu, Feb 04, 2021 at 05:56:50PM +0100, Greg Kroah-Hartman wrote: > > On Thu, Feb 04, 2021 at 04:52:24PM +, Russell King - ARM Linux admin > > wrote: > > > On Tue, Feb 02, 2021 at 03:06:05PM +0100, Greg Kroah-Hartman wrote: > > > > I'm glad to take this through my char/misc tree, as that's where the > > > > other coresight changes flow through. So if no one else objects, I will > > > > do so... > > > > > > Greg, did you end up pulling this after all? If not, Uwe produced a v2. > > > I haven't merged v2 yet as I don't know what you've done. > > > > I thought you merged this? > > I took v1, and put it in a branch I've promised in the past not to > rebase/rewind. Uwe is now asking for me to take a v2 or apply a patch > on top. > > The only reason to produce an "immutable" branch is if it's the basis > for some dependent work and you need that branch merged into other > people's trees... so the whole "lets produce a v2" is really odd > workflow... I'm confused about what I should do, and who has to be > informed which option I take. > > I'm rather lost here too. Sorry to have cause this confusion. After I saw that my initial tag missed to adapt a driver I wanted to make it easy for you to fix the situation. So I created a patch to fix it and created a second tag with the patch squashed in. Obviously only one of them have to be picked and I hoped you (= Russell + Greg) would agree which option to pick. My preference would be if you both pick up v2 of the tag to yield a history that is bisectable without build problems, but if Russell (who already picked up the broken tag) considers his tree immutable and so isn't willing to rebase, then picking up the patch is the way to go. I suggest that Russell descides which option he wants to pick and tells Greg to do the same!? Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König| Industrial Linux Solutions | https://www.pengutronix.de/ | signature.asc Description: PGP signature
Re: [PATCH v3 0/5] amba: minor fix and various cleanups
Hello, we already talked about this via irc, but for the record and the benefit of others: On Tue, Feb 02, 2021 at 10:49:15AM +, Russell King - ARM Linux admin wrote: > I think you need to have a 6th patch which moves the > probe/remove/shutdown methods into the bus_type - if you're setting > them for every struct device_driver, then there's no point doing that > and they may as well be in the bus_type. This is implemented in patch 5 already. Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König| Industrial Linux Solutions | https://www.pengutronix.de/ | signature.asc Description: PGP signature
Re: [GIT PULL] immutable branch for amba changes targeting v5.12-rc1
Hello,
On Tue, Feb 02, 2021 at 02:53:50PM +0100, Uwe Kleine-König wrote:
> the following changes since commit 5c8fe583cce542aa0b84adc939ce85293de36e5e:
>
> Linux 5.11-rc1 (2020-12-27 15:30:22 -0800)
>
> are available in the Git repository at:
>
> https://git.pengutronix.de/git/ukl/linux tags/amba-make-remove-return-void
>
> for you to fetch changes up to f170b59fedd733b92f58c4d7c8357fbf7601d623:
>
> amba: Make use of bus_type functions (2021-02-02 14:26:02 +0100)
>
> I expect this tag to be merged by Russell King as amba maintainer and by
> Mathieu Poirier (or Greg Kroah-Hartman?) for coresight as there are some
> pending conflicting changes. These are not hard to resolve but also
> non-trivial. Tell me if you need assistance for resolving, also if it's only a
> second pair of eyes to judge your resolution.
Alternatively to my additional patch sent earlier today I prepared a v2
tag at
https://git.pengutronix.de/git/ukl/linux tags/amba-make-remove-return-void-v2
with the build fix squashed in. Iff you prefer and both Russell and Greg
agree to pull this one instead of the (implicit) v1 we can maybe prevent
introducing this build regression in mainline. Please coordinate among
you two. And sorry again for breaking the build.
Best regards
Uwe
PS: The range-diff between the original
tags/amba-make-remove-return-void and
tags/amba-make-remove-return-void-v2 is:
1: 3fd269e74f2f ! 1: 481963c91284 amba: Make the remove callback return void
@@ Commit message
Acked-by: Vladimir Zapolskiy # for memory/pl172
Acked-by: Greg Kroah-Hartman
Link:
https://lore.kernel.org/r/20210126165835.687514-5-u.kleine-koe...@pengutronix.de
+[ukleinek: squashed in a build fix for drivers/mailbox/arm_mhuv2.c]
Signed-off-by: Uwe Kleine-König
## drivers/amba/bus.c ##
@@ drivers/input/serio/ambakmi.c: static int amba_kmi_remove(struct
amba_device *de
static int __maybe_unused amba_kmi_resume(struct device *dev)
+ ## drivers/mailbox/arm_mhuv2.c ##
+@@ drivers/mailbox/arm_mhuv2.c: static int mhuv2_probe(struct amba_device
*adev, const struct amba_id *id)
+ return ret;
+ }
+
+-static int mhuv2_remove(struct amba_device *adev)
++static void mhuv2_remove(struct amba_device *adev)
+ {
+ struct mhuv2 *mhu = amba_get_drvdata(adev);
+
+ if (mhu->frame == SENDER_FRAME)
+ writel_relaxed(0x0, &mhu->send->access_request);
+-
+- return 0;
+ }
+
+ static struct amba_id mhuv2_ids[] = {
+
## drivers/memory/pl172.c ##
@@ drivers/memory/pl172.c: static int pl172_probe(struct amba_device
*adev, const struct amba_id *id)
return ret;
2: f170b59fedd7 = 2: f30d22a7bfab amba: Make use of bus_type functions
--
Pengutronix e.K. | Uwe Kleine-König|
Industrial Linux Solutions | https://www.pengutronix.de/ |
signature.asc
Description: PGP signature
[PATCH] mailbox: arm_mhuv2: make remove callback return void
My build tests failed to catch that amba driver that would have needed
adaption in commit 3fd269e74f2f ("amba: Make the remove callback return
void"). Change the remove function to make the driver build again.
Reported-by: kernel test robot
Fixes: 3fd269e74f2f ("amba: Make the remove callback return void")
Signed-off-by: Uwe Kleine-König
---
Hello,
I guess I missed that driver during rebase as it was only introduced in
the last merge window. Sorry for that.
I'm unsure what is the right thing to do now. Should I redo the pull
request (with this patch squashed into 3fd269e74f2f)? Or do we just
apply this patch on top?
FTR, the test robot report is at
https://lore.kernel.org/r/202102030343.d9j1wukx-...@intel.com
Best regards
Uwe
drivers/mailbox/arm_mhuv2.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/mailbox/arm_mhuv2.c b/drivers/mailbox/arm_mhuv2.c
index 67fb10885bb4..6cf1991a5c9c 100644
--- a/drivers/mailbox/arm_mhuv2.c
+++ b/drivers/mailbox/arm_mhuv2.c
@@ -1095,14 +1095,12 @@ static int mhuv2_probe(struct amba_device *adev, const
struct amba_id *id)
return ret;
}
-static int mhuv2_remove(struct amba_device *adev)
+static void mhuv2_remove(struct amba_device *adev)
{
struct mhuv2 *mhu = amba_get_drvdata(adev);
if (mhu->frame == SENDER_FRAME)
writel_relaxed(0x0, &mhu->send->access_request);
-
- return 0;
}
static struct amba_id mhuv2_ids[] = {
--
2.29.2
signature.asc
Description: PGP signature
[GIT PULL] immutable branch for amba changes targeting v5.12-rc1
Hello, the following changes since commit 5c8fe583cce542aa0b84adc939ce85293de36e5e: Linux 5.11-rc1 (2020-12-27 15:30:22 -0800) are available in the Git repository at: https://git.pengutronix.de/git/ukl/linux tags/amba-make-remove-return-void for you to fetch changes up to f170b59fedd733b92f58c4d7c8357fbf7601d623: amba: Make use of bus_type functions (2021-02-02 14:26:02 +0100) I expect this tag to be merged by Russell King as amba maintainer and by Mathieu Poirier (or Greg Kroah-Hartman?) for coresight as there are some pending conflicting changes. These are not hard to resolve but also non-trivial. Tell me if you need assistance for resolving, also if it's only a second pair of eyes to judge your resolution. Best regards, Uwe Tag for adaptions to struct amba_driver::remove changing prototype ---- Uwe Kleine-König (5): amba: Fix resource leak for drivers without .remove amba: reorder functions vfio: platform: simplify device removal amba: Make the remove callback return void amba: Make use of bus_type functions drivers/amba/bus.c | 234 +-- drivers/char/hw_random/nomadik-rng.c | 3 +- drivers/dma/pl330.c| 3 +- drivers/gpu/drm/pl111/pl111_drv.c | 4 +- drivers/hwtracing/coresight/coresight-catu.c | 3 +- drivers/hwtracing/coresight/coresight-cpu-debug.c | 4 +- drivers/hwtracing/coresight/coresight-cti-core.c | 4 +- drivers/hwtracing/coresight/coresight-etb10.c | 4 +- drivers/hwtracing/coresight/coresight-etm3x-core.c | 4 +- drivers/hwtracing/coresight/coresight-etm4x-core.c | 4 +- drivers/hwtracing/coresight/coresight-funnel.c | 4 +- drivers/hwtracing/coresight/coresight-replicator.c | 4 +- drivers/hwtracing/coresight/coresight-stm.c| 4 +- drivers/hwtracing/coresight/coresight-tmc-core.c | 4 +- drivers/hwtracing/coresight/coresight-tpiu.c | 4 +- drivers/i2c/busses/i2c-nomadik.c | 4 +- drivers/input/serio/ambakmi.c | 3 +- drivers/memory/pl172.c | 4 +- drivers/memory/pl353-smc.c | 4 +- drivers/mmc/host/mmci.c| 4 +- drivers/rtc/rtc-pl030.c| 4 +- drivers/rtc/rtc-pl031.c| 4 +- drivers/spi/spi-pl022.c| 5 +- drivers/tty/serial/amba-pl010.c| 4 +- drivers/tty/serial/amba-pl011.c| 3 +- drivers/vfio/platform/vfio_amba.c | 15 ++-- drivers/video/fbdev/amba-clcd.c| 4 +- drivers/watchdog/sp805_wdt.c | 4 +- include/linux/amba/bus.h | 2 +- sound/arm/aaci.c | 4 +- 30 files changed, 157 insertions(+), 198 deletions(-) signature.asc Description: PGP signature
Re: [PATCH] vio: make remove callback return void
Hello Sukadev, On 1/28/21 8:07 PM, Sukadev Bhattiprolu wrote: Slightly off-topic, should ndo_stop() also return a void? Its return value seems to be mostly ignored and [...] I don't know enough about the network stack to tell. Probably it's a good idea to start a separate thread for this and address this to the netdev list only. Best regards Uwe OpenPGP_signature Description: OpenPGP digital signature
[PATCH] vio: make remove callback return void
The driver core ignores the return value of struct bus_type::remove()
because there is only little that can be done. To simplify the quest to
make this function return void, let struct vio_driver::remove() return
void, too. All users already unconditionally return 0, this commit makes
it obvious that returning an error code is a bad idea and makes it
obvious for future driver authors that returning an error code isn't
intended.
Note there are two nominally different implementations for a vio bus:
one in arch/sparc/kernel/vio.c and the other in
arch/powerpc/platforms/pseries/vio.c. I didn't care to check which
driver is using which of these busses (or if even some of them can be
used with both) and simply adapt all drivers and the two bus codes in
one go.
Note that for the powerpc implementation there is a semantical change:
Before this patch for a device that was bound to a driver without a
remove callback vio_cmo_bus_remove(viodev) wasn't called. As the device
core still considers the device unbound after vio_bus_remove() returns
calling this unconditionally is the consistent behaviour which is
implemented here.
Signed-off-by: Uwe Kleine-König
---
Hello,
note that this change depends on
https://lore.kernel.org/r/20210121062005.53271-1-...@linux.ibm.com which removes
an (ignored) return -EBUSY in drivers/net/ethernet/ibm/ibmvnic.c.
I don't know when/if this latter patch will be applied, so it might take
some time until my patch can go in.
Best regards
Uwe
arch/powerpc/include/asm/vio.h | 2 +-
arch/powerpc/platforms/pseries/vio.c | 7 +++
arch/sparc/include/asm/vio.h | 2 +-
arch/sparc/kernel/ds.c | 6 --
arch/sparc/kernel/vio.c | 4 ++--
drivers/block/sunvdc.c | 3 +--
drivers/char/hw_random/pseries-rng.c | 3 +--
drivers/char/tpm/tpm_ibmvtpm.c | 4 +---
drivers/crypto/nx/nx-842-pseries.c | 4 +---
drivers/crypto/nx/nx.c | 4 +---
drivers/misc/ibmvmc.c| 4 +---
drivers/net/ethernet/ibm/ibmveth.c | 4 +---
drivers/net/ethernet/ibm/ibmvnic.c | 4 +---
drivers/net/ethernet/sun/ldmvsw.c| 4 +---
drivers/net/ethernet/sun/sunvnet.c | 3 +--
drivers/scsi/ibmvscsi/ibmvfc.c | 3 +--
drivers/scsi/ibmvscsi/ibmvscsi.c | 4 +---
drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c | 4 +---
drivers/tty/hvc/hvcs.c | 3 +--
drivers/tty/vcc.c| 4 +---
20 files changed, 22 insertions(+), 54 deletions(-)
diff --git a/arch/powerpc/include/asm/vio.h b/arch/powerpc/include/asm/vio.h
index 0cf52746531b..721c0d6715ac 100644
--- a/arch/powerpc/include/asm/vio.h
+++ b/arch/powerpc/include/asm/vio.h
@@ -113,7 +113,7 @@ struct vio_driver {
const char *name;
const struct vio_device_id *id_table;
int (*probe)(struct vio_dev *dev, const struct vio_device_id *id);
- int (*remove)(struct vio_dev *dev);
+ void (*remove)(struct vio_dev *dev);
/* A driver must have a get_desired_dma() function to
* be loaded in a CMO environment if it uses DMA.
*/
diff --git a/arch/powerpc/platforms/pseries/vio.c
b/arch/powerpc/platforms/pseries/vio.c
index b2797cfe4e2b..9cb4fc839fd5 100644
--- a/arch/powerpc/platforms/pseries/vio.c
+++ b/arch/powerpc/platforms/pseries/vio.c
@@ -1261,7 +1261,6 @@ static int vio_bus_remove(struct device *dev)
struct vio_dev *viodev = to_vio_dev(dev);
struct vio_driver *viodrv = to_vio_driver(dev->driver);
struct device *devptr;
- int ret = 1;
/*
* Hold a reference to the device after the remove function is called
@@ -1270,13 +1269,13 @@ static int vio_bus_remove(struct device *dev)
devptr = get_device(dev);
if (viodrv->remove)
- ret = viodrv->remove(viodev);
+ viodrv->remove(viodev);
- if (!ret && firmware_has_feature(FW_FEATURE_CMO))
+ if (firmware_has_feature(FW_FEATURE_CMO))
vio_cmo_bus_remove(viodev);
put_device(devptr);
- return ret;
+ return 0;
}
/**
diff --git a/arch/sparc/include/asm/vio.h b/arch/sparc/include/asm/vio.h
index 059f0eb678e0..8a1a83bbb6d5 100644
--- a/arch/sparc/include/asm/vio.h
+++ b/arch/sparc/include/asm/vio.h
@@ -362,7 +362,7 @@ struct vio_driver {
struct list_headnode;
const struct vio_device_id *id_table;
int (*probe)(struct vio_dev *dev, const struct vio_device_id *id);
- int (*remove)(struct vio_dev *dev);
+ void (*remove)(struct vio_dev *dev);
void (*shutdown)(struct vio_dev *dev);
unsigned long driver_data;
struct device_driverdriver;
diff --git a/arch/sparc/kernel/ds.c b/arch/sparc/kernel/ds.c
index 522e5b51050c..4a5bdb0df779 100644
--- a/arch/sparc/kernel/ds.c
+++ b/arch/sparc/kernel/ds.c
@@ -1236,11 +1236
Re: [PATCH v3 4/5] amba: Make the remove callback return void
Hello,
On Tue, Jan 26, 2021 at 05:08:40PM +, Suzuki K Poulose wrote:
> On 1/26/21 4:58 PM, Uwe Kleine-König wrote:
> > All amba drivers return 0 in their remove callback. Together with the
> > driver core ignoring the return value anyhow, it doesn't make sense to
> > return a value here.
> >
> > Change the remove prototype to return void, which makes it explicit that
> > returning an error value doesn't work as expected. This simplifies changing
> > the core remove callback to return void, too.
> >
> > Reviewed-by: Ulf Hansson
> > Reviewed-by: Arnd Bergmann
> > Acked-by: Alexandre Belloni
> > Acked-by: Dmitry Torokhov
> > Acked-by: Krzysztof Kozlowski # for drivers/memory
> > Acked-by: Mark Brown
> > Acked-by: Dmitry Torokhov
> > Acked-by: Linus Walleij
> > Signed-off-by: Uwe Kleine-König
>
>
> > drivers/hwtracing/coresight/coresight-etm4x-core.c | 4 +---
>
> You are most likely to have a conflict for the above file, with what is
> in coresight/next. It should be easy to resolve.
I'm surprised to see that the remove callback introduced in 2952ecf5df33
("coresight: etm4x: Refactor probing routine") has an __exit annotation.
With .suppress_bind_attrs = true you don't need a remove callback at
all. (And without .suppress_bind_attrs = true the remove callback must
have no __exit annotation.)
This make me looking at commit 45fe7befe0db ("coresight: remove broken
__exit annotations") by Arnd. Unless I miss something the better change
would have been to remove the unused remove callbacks instead of dropping
their __exit annotation?!
Anyhow, my conflict resolution looks as follows:
diff --cc drivers/hwtracing/coresight/coresight-etm4x-core.c
index 82787cba537d,473ab7480a36..
--- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
@@@ -1703,6 -1903,28 +1903,27 @@@ static int __exit etm4_remove_dev(struc
cpus_read_unlock();
coresight_unregister(drvdata->csdev);
+
+ return 0;
+ }
+
-static int __exit etm4_remove_amba(struct amba_device *adev)
++static void __exit etm4_remove_amba(struct amba_device *adev)
+ {
+ struct etmv4_drvdata *drvdata = dev_get_drvdata(&adev->dev);
+
+ if (drvdata)
- return etm4_remove_dev(drvdata);
- return 0;
++ etm4_remove_dev(drvdata);
+ }
+
+ static int __exit etm4_remove_platform_dev(struct platform_device *pdev)
+ {
+ int ret = 0;
+ struct etmv4_drvdata *drvdata = dev_get_drvdata(&pdev->dev);
+
+ if (drvdata)
+ ret = etm4_remove_dev(drvdata);
+ pm_runtime_disable(&pdev->dev);
+ return ret;
}
static const struct amba_id etm4_ids[] = {
If this series should make it in for 5.12 we probably need an immutable
branch between hwtracing and amba.
> Otherwise, the changes look good for the drivers/hwtracing/coresight/*
>
> Acked-by: Suzuki K Poulose
Thanks
Uwe
--
Pengutronix e.K. | Uwe Kleine-König|
Industrial Linux Solutions | https://www.pengutronix.de/ |
signature.asc
Description: PGP signature
[PATCH v3 4/5] amba: Make the remove callback return void
All amba drivers return 0 in their remove callback. Together with the
driver core ignoring the return value anyhow, it doesn't make sense to
return a value here.
Change the remove prototype to return void, which makes it explicit that
returning an error value doesn't work as expected. This simplifies changing
the core remove callback to return void, too.
Reviewed-by: Ulf Hansson
Reviewed-by: Arnd Bergmann
Acked-by: Alexandre Belloni
Acked-by: Dmitry Torokhov
Acked-by: Krzysztof Kozlowski # for drivers/memory
Acked-by: Mark Brown
Acked-by: Dmitry Torokhov
Acked-by: Linus Walleij
Signed-off-by: Uwe Kleine-König
---
drivers/amba/bus.c | 5 ++---
drivers/char/hw_random/nomadik-rng.c | 3 +--
drivers/dma/pl330.c| 3 +--
drivers/gpu/drm/pl111/pl111_drv.c | 4 +---
drivers/hwtracing/coresight/coresight-catu.c | 3 +--
drivers/hwtracing/coresight/coresight-cpu-debug.c | 4 +---
drivers/hwtracing/coresight/coresight-cti-core.c | 4 +---
drivers/hwtracing/coresight/coresight-etb10.c | 4 +---
drivers/hwtracing/coresight/coresight-etm3x-core.c | 4 +---
drivers/hwtracing/coresight/coresight-etm4x-core.c | 4 +---
drivers/hwtracing/coresight/coresight-funnel.c | 4 ++--
drivers/hwtracing/coresight/coresight-replicator.c | 4 ++--
drivers/hwtracing/coresight/coresight-stm.c| 4 +---
drivers/hwtracing/coresight/coresight-tmc-core.c | 4 +---
drivers/hwtracing/coresight/coresight-tpiu.c | 4 +---
drivers/i2c/busses/i2c-nomadik.c | 4 +---
drivers/input/serio/ambakmi.c | 3 +--
drivers/memory/pl172.c | 4 +---
drivers/memory/pl353-smc.c | 4 +---
drivers/mmc/host/mmci.c| 4 +---
drivers/rtc/rtc-pl030.c| 4 +---
drivers/rtc/rtc-pl031.c| 4 +---
drivers/spi/spi-pl022.c| 5 ++---
drivers/tty/serial/amba-pl010.c| 4 +---
drivers/tty/serial/amba-pl011.c| 3 +--
drivers/vfio/platform/vfio_amba.c | 3 +--
drivers/video/fbdev/amba-clcd.c| 4 +---
drivers/watchdog/sp805_wdt.c | 4 +---
include/linux/amba/bus.h | 2 +-
sound/arm/aaci.c | 4 +---
30 files changed, 34 insertions(+), 80 deletions(-)
diff --git a/drivers/amba/bus.c b/drivers/amba/bus.c
index 8c4a42df47c6..48b5d4b4e889 100644
--- a/drivers/amba/bus.c
+++ b/drivers/amba/bus.c
@@ -300,11 +300,10 @@ static int amba_remove(struct device *dev)
{
struct amba_device *pcdev = to_amba_device(dev);
struct amba_driver *drv = to_amba_driver(dev->driver);
- int ret = 0;
pm_runtime_get_sync(dev);
if (drv->remove)
- ret = drv->remove(pcdev);
+ drv->remove(pcdev);
pm_runtime_put_noidle(dev);
/* Undo the runtime PM settings in amba_probe() */
@@ -315,7 +314,7 @@ static int amba_remove(struct device *dev)
amba_put_disable_pclk(pcdev);
dev_pm_domain_detach(dev, true);
- return ret;
+ return 0;
}
static void amba_shutdown(struct device *dev)
diff --git a/drivers/char/hw_random/nomadik-rng.c
b/drivers/char/hw_random/nomadik-rng.c
index b0ded41eb865..67947a19aa22 100644
--- a/drivers/char/hw_random/nomadik-rng.c
+++ b/drivers/char/hw_random/nomadik-rng.c
@@ -69,11 +69,10 @@ static int nmk_rng_probe(struct amba_device *dev, const
struct amba_id *id)
return ret;
}
-static int nmk_rng_remove(struct amba_device *dev)
+static void nmk_rng_remove(struct amba_device *dev)
{
amba_release_regions(dev);
clk_disable(rng_clk);
- return 0;
}
static const struct amba_id nmk_rng_ids[] = {
diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
index bc0f66af0f11..fd8d2bc3be9f 100644
--- a/drivers/dma/pl330.c
+++ b/drivers/dma/pl330.c
@@ -3195,7 +3195,7 @@ pl330_probe(struct amba_device *adev, const struct
amba_id *id)
return ret;
}
-static int pl330_remove(struct amba_device *adev)
+static void pl330_remove(struct amba_device *adev)
{
struct pl330_dmac *pl330 = amba_get_drvdata(adev);
struct dma_pl330_chan *pch, *_p;
@@ -3235,7 +3235,6 @@ static int pl330_remove(struct amba_device *adev)
if (pl330->rstc)
reset_control_assert(pl330->rstc);
- return 0;
}
static const struct amba_id pl330_ids[] = {
diff --git a/drivers/gpu/drm/pl111/pl111_drv.c
b/drivers/gpu/drm/pl111/pl111_drv.c
index 40e6708fbbe2..1fb5eacefd2d 100644
--- a/drivers/gpu/drm/pl111/pl111_drv.c
+++ b/drivers/gpu/drm/pl111/pl111_drv.c
@@ -320,7 +320,7 @@ static int pl111_amba_probe(struct amba_device *amba_dev,
return ret;
}
-static int pl111_amba_remove(struct amba_device *amba_dev)
+s
[PATCH v3 0/5] amba: minor fix and various cleanups
From: Uwe Kleine-König
Re: [PATCH 3/3] hwrng: mxc-fsl - add support for Freescale RNGC
Hello Fabio,
On Mon, Feb 29, 2016 at 08:54:19PM -0300, Fabio Estevam wrote:
> On Mon, Feb 29, 2016 at 6:38 PM, Uwe Kleine-König
> wrote:
> > On Mon, Feb 29, 2016 at 06:16:50PM -0300, Fabio Estevam wrote:
> >> On Mon, Feb 29, 2016 at 12:52 PM, Steffen Trumtrar
> >> wrote:
> >>
> >> > + ret = clk_prepare_enable(rngc->clk);
> >> > + if (ret)
> >> > + return ret;
> >> > +
> >> > + rngc->irq = platform_get_irq(pdev, 0);
> >> > + if (!rngc->irq) {
> >> > + dev_err(&pdev->dev, "FSL RNGC couldn't get irq\n");
> >> > + clk_disable_unprepare(rngc->clk);
> >> > +
> >> > + return ret;
> >>
> >> You are returning the wrong error code here:
> >>
> >> Better do like this:
> >>
> >>rngc->irq = platform_get_irq(pdev, 0);
> >>if (rngc->irq < 0) {
> >
> > rngc->irq is unsigned, so this is never true.
> >
> >>dev_err(&pdev->dev, "FSL RNGC couldn't get irq\n");
> >>clk_disable_unprepare(rngc->clk);
> >>return rngc->irq;
> >>}
> >
> > So here comes my better approach:
>
> As irq is only used inside probe it can be removed from struct mxc_rngc.
Good idea.
> Or maybe like this:
>
> ret = platform_get_irq(pdev, 0);
> if (ret < 0) {
> dev_err(&pdev->dev, "FSL RNGC couldn't get irq\n");
> clk_disable_unprepare(rngc->clk);
> return ret;
> }
Some people think platform_get_irq returning 0 should be handled as
error.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König|
Industrial Linux Solutions | http://www.pengutronix.de/ |
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] hwrng: mxc-fsl - add support for Freescale RNGC
On Mon, Feb 29, 2016 at 06:16:50PM -0300, Fabio Estevam wrote:
> On Mon, Feb 29, 2016 at 12:52 PM, Steffen Trumtrar
> wrote:
>
> > + ret = clk_prepare_enable(rngc->clk);
> > + if (ret)
> > + return ret;
> > +
> > + rngc->irq = platform_get_irq(pdev, 0);
> > + if (!rngc->irq) {
> > + dev_err(&pdev->dev, "FSL RNGC couldn't get irq\n");
> > + clk_disable_unprepare(rngc->clk);
> > +
> > + return ret;
>
> You are returning the wrong error code here:
>
> Better do like this:
>
>rngc->irq = platform_get_irq(pdev, 0);
>if (rngc->irq < 0) {
rngc->irq is unsigned, so this is never true.
>dev_err(&pdev->dev, "FSL RNGC couldn't get irq\n");
>clk_disable_unprepare(rngc->clk);
>return rngc->irq;
>}
So here comes my better approach:
ret = platform_get_irq(pdev, 0);
if (ret <= 0) {
if (ret == 0)
ret = -EINVAL;
dev_err(&pdev->dev, "FSL RNGC couldn't get irq\n");
clk_disable_unprepare(rngc->clk);
return ret;
}
rngc->irq = ret;
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König|
Industrial Linux Solutions | http://www.pengutronix.de/ |
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 2/5] crypto: sunxi-ss: fix min3() call to match types
On Tue, Dec 22, 2015 at 12:27:44PM +, Andre Przywara wrote:
> The min3() macro expects all arguments to be of the same type (or
> size at least). While two arguments are ints or u32s, one is size_t,
> which does not match on 64-bit architectures.
> Cast the size_t to u32 to make min3() happy. In this context here the
> length should never exceed 32 bits anyway.
>
> Signed-off-by: Andre Przywara
> ---
> drivers/crypto/sunxi-ss/sun4i-ss-cipher.c | 12 ++--
> drivers/crypto/sunxi-ss/sun4i-ss-hash.c | 8
> 2 files changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/crypto/sunxi-ss/sun4i-ss-cipher.c
> b/drivers/crypto/sunxi-ss/sun4i-ss-cipher.c
> index a19ee12..b3bc7bd 100644
> --- a/drivers/crypto/sunxi-ss/sun4i-ss-cipher.c
> +++ b/drivers/crypto/sunxi-ss/sun4i-ss-cipher.c
> @@ -79,7 +79,7 @@ static int sun4i_ss_opti_poll(struct ablkcipher_request
> *areq)
> oi = 0;
> oo = 0;
> do {
> - todo = min3(rx_cnt, ileft, (mi.length - oi) / 4);
> + todo = min3(rx_cnt, ileft, (u32)(mi.length - oi) / 4);
For this case the min function has a min_t variant to specify the
argument. What about introducing min3_t?
BTW, I don't understand why min3(x, y, z) isn't just defined as
#define min3(x, y, z) min(min(x, y), z)
but instead as:
#define min3(x, y, z) min((typeof(x))min(x, y), z)
. I thought min(x, y) has the same type as x anyhow?
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König|
Industrial Linux Solutions | http://www.pengutronix.de/ |
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
