| Message ID | 5107AE6F.3080206@asianux.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Jan 29, 2013 at 07:11:43PM +0800, Chen Gang wrote: > need set NULL before return, just like function sa1111_remove has done. > and better to use sa1111_remove directly, instead of current implementation. NAK. 1. __sa1111_remove() will kfree sachip, so the value of sachip->saved_state at this point is meaningless. 2. I don't think you tried to build with your patch in place.
? 2013?01?29? 19:15, Russell King - ARM Linux ??: > On Tue, Jan 29, 2013 at 07:11:43PM +0800, Chen Gang wrote: >> > need set NULL before return, just like function sa1111_remove has done. >> > and better to use sa1111_remove directly, instead of current implementation. > NAK. > > 1. __sa1111_remove() will kfree sachip, so the value of sachip->saved_state > at this point is meaningless. yes, just like what you said. it is not a bug (it seems not necessary to set saved_state to NULL in sa1111_remove, either). I think, this patch can make the source code simpler and clearer. so I still suggest to apply it (although it seems a minor patch). > 2. I don't think you tried to build with your patch in place. > > it is my fault, just like what you said, I did not try to build. and I should try now. thanks.
it is my fault, building failed for not declare sa1111_remove before using.
I should notice next time (at least, need building).
if sending patch v2 is welcome, I will do.
if need additional test, I should try (I try under qemu virtual machine).
:-)
gchen.
? 2013?01?29? 19:34, Chen Gang ??:
>> > 2. I don't think you tried to build with your patch in place.
>> >
>> >
> it is my fault, just like what you said, I did not try to build.
> and I should try now.
diff --git a/arch/arm/common/sa1111.c b/arch/arm/common/sa1111.c index e57d7e5..4bb8230 100644 --- a/arch/arm/common/sa1111.c +++ b/arch/arm/common/sa1111.c @@ -954,9 +954,7 @@ static int sa1111_resume(struct platform_device *dev) */ id = sa1111_readl(sachip->base + SA1111_SKID); if ((id & SKID_ID_MASK) != SKID_SA1111_ID) { - __sa1111_remove(sachip); - platform_set_drvdata(dev, NULL); - kfree(save); + sa1111_remove(dev); return 0; }
need set NULL before return, just like function sa1111_remove has done. and better to use sa1111_remove directly, instead of current implementation. Signed-off-by: Chen Gang <gang.chen@asianux.com> --- arch/arm/common/sa1111.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)