v2:
- fix some problems and split v1 patch into this patch set.(Thanks to
Dan)
v1:
- https://lore.kernel.org/all/[email protected]/
Su Hui (3):
scsi: aic7xxx: return negative error codes in
ahc_linux_register_host()
scsi: aic7xxx: return ahc_linux_register_host()'s value rather than
zero
scsi: aic7xxx: return negative error codes in aic7770_probe()
drivers/scsi/aic7xxx/aic7770_osm.c | 6 +++---
drivers/scsi/aic7xxx/aic7xxx_osm.c | 2 +-
drivers/scsi/aic7xxx/aic7xxx_osm_pci.c | 3 +--
3 files changed, 5 insertions(+), 6 deletions(-)
--
2.30.2
aic7770_config() returns both negative and positive error code.
it's better to make aic7770_probe() only return negative error codes.
And the previous patch made ahc_linux_register_host() return negative error
codes, which makes sure aic7770_probe() returns negative error codes.
Signed-off-by: Su Hui <[email protected]>
---
drivers/scsi/aic7xxx/aic7770_osm.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/scsi/aic7xxx/aic7770_osm.c b/drivers/scsi/aic7xxx/aic7770_osm.c
index bdd177e3d762..a19cdd87c453 100644
--- a/drivers/scsi/aic7xxx/aic7770_osm.c
+++ b/drivers/scsi/aic7xxx/aic7770_osm.c
@@ -87,17 +87,17 @@ aic7770_probe(struct device *dev)
sprintf(buf, "ahc_eisa:%d", eisaBase >> 12);
name = kstrdup(buf, GFP_ATOMIC);
if (name == NULL)
- return (ENOMEM);
+ return -ENOMEM;
ahc = ahc_alloc(&aic7xxx_driver_template, name);
if (ahc == NULL)
- return (ENOMEM);
+ return -ENOMEM;
ahc->dev = dev;
error = aic7770_config(ahc, aic7770_ident_table + edev->id.driver_data,
eisaBase);
if (error != 0) {
ahc->bsh.ioport = 0;
ahc_free(ahc);
- return (error);
+ return error < 0 ? error : -error;
}
dev_set_drvdata(dev, ahc);
--
2.30.2
On Fri, Dec 01, 2023 at 10:59:53AM +0800, Su Hui wrote:
> v2:
> - fix some problems and split v1 patch into this patch set.(Thanks to
> Dan)
>
> v1:
> - https://lore.kernel.org/all/[email protected]/
>
Would have been better with Fixes tags probably. Otherwise, it looks
good to me.
Reviewed-by: Dan Carpenter <[email protected]>
regards,
dan carpenter
Le 01/12/2023 à 03:59, Su Hui a écrit :
> aic7770_config() returns both negative and positive error code.
> it's better to make aic7770_probe() only return negative error codes.
>
> And the previous patch made ahc_linux_register_host() return negative error
> codes, which makes sure aic7770_probe() returns negative error codes.
>
> Signed-off-by: Su Hui <[email protected]>
> ---
> drivers/scsi/aic7xxx/aic7770_osm.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/scsi/aic7xxx/aic7770_osm.c b/drivers/scsi/aic7xxx/aic7770_osm.c
> index bdd177e3d762..a19cdd87c453 100644
> --- a/drivers/scsi/aic7xxx/aic7770_osm.c
> +++ b/drivers/scsi/aic7xxx/aic7770_osm.c
> @@ -87,17 +87,17 @@ aic7770_probe(struct device *dev)
> sprintf(buf, "ahc_eisa:%d", eisaBase >> 12);
> name = kstrdup(buf, GFP_ATOMIC);
> if (name == NULL)
> - return (ENOMEM);
> + return -ENOMEM;
> ahc = ahc_alloc(&aic7xxx_driver_template, name);
> if (ahc == NULL)
Unrelated to your fix, but 'name' is leaking here.
Also, kasprintf() could be used to avoid buf+sprintf()+kstrdup()
The GFP_ATOMIC in the allocation could certainly also be just a GFP_KERNEL.
CJ
> - return (ENOMEM);
> + return -ENOMEM;
> ahc->dev = dev;
> error = aic7770_config(ahc, aic7770_ident_table + edev->id.driver_data,
> eisaBase);
> if (error != 0) {
> ahc->bsh.ioport = 0;
> ahc_free(ahc);
> - return (error);
> + return error < 0 ? error : -error;
> }
>
> dev_set_drvdata(dev, ahc);
Le 03/12/2023 à 11:34, Christophe JAILLET a écrit :
> Le 01/12/2023 à 03:59, Su Hui a écrit :
>> aic7770_config() returns both negative and positive error code.
>> it's better to make aic7770_probe() only return negative error codes.
>>
>> And the previous patch made ahc_linux_register_host() return negative
>> error
>> codes, which makes sure aic7770_probe() returns negative error codes.
>>
>> Signed-off-by: Su Hui <[email protected]>
>> ---
>> drivers/scsi/aic7xxx/aic7770_osm.c | 6 +++---
>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/scsi/aic7xxx/aic7770_osm.c
>> b/drivers/scsi/aic7xxx/aic7770_osm.c
>> index bdd177e3d762..a19cdd87c453 100644
>> --- a/drivers/scsi/aic7xxx/aic7770_osm.c
>> +++ b/drivers/scsi/aic7xxx/aic7770_osm.c
>> @@ -87,17 +87,17 @@ aic7770_probe(struct device *dev)
>> sprintf(buf, "ahc_eisa:%d", eisaBase >> 12);
>> name = kstrdup(buf, GFP_ATOMIC);
>> if (name == NULL)
>> - return (ENOMEM);
>> + return -ENOMEM;
>> ahc = ahc_alloc(&aic7xxx_driver_template, name);
>> if (ahc == NULL)
>
> Unrelated to your fix, but 'name' is leaking here.
Oups, no, ahc_alloc() handles it.
Really strange API!
CJ
>
> Also, kasprintf() could be used to avoid buf+sprintf()+kstrdup()
>
> The GFP_ATOMIC in the allocation could certainly also be just a GFP_KERNEL.
>
> CJ
>
>> - return (ENOMEM);
>> + return -ENOMEM;
>> ahc->dev = dev;
>> error = aic7770_config(ahc, aic7770_ident_table +
>> edev->id.driver_data,
>> eisaBase);
>> if (error != 0) {
>> ahc->bsh.ioport = 0;
>> ahc_free(ahc);
>> - return (error);
>> + return error < 0 ? error : -error;
>> }
>> dev_set_drvdata(dev, ahc);
>
>
On 2023/12/1 15:53, Dan Carpenter wrote:
> On Fri, Dec 01, 2023 at 10:59:53AM +0800, Su Hui wrote:
>> v2:
>> - fix some problems and split v1 patch into this patch set.(Thanks to
>> Dan)
>>
>> v1:
>> - https://lore.kernel.org/all/[email protected]/
>>
> Would have been better with Fixes tags probably. Otherwise, it looks
> good to me.
Hi, Dan
Sorry for the late reply.
I'm not sure if it's worth to add Fixes tags.
These codes are very old which come from "Linux-2.6.12-rc2".
It's seems like a cleanup or improvement.
Umm, should I send v3 patches to add Fixes tags?
Su Hui
On Tue, Dec 05, 2023 at 11:33:36AM +0800, Su Hui wrote:
> On 2023/12/1 15:53, Dan Carpenter wrote:
> > On Fri, Dec 01, 2023 at 10:59:53AM +0800, Su Hui wrote:
> > > v2:
> > > - fix some problems and split v1 patch into this patch set.(Thanks to
> > > Dan)
> > >
> > > v1:
> > > - https://lore.kernel.org/all/[email protected]/
> > >
> > Would have been better with Fixes tags probably. Otherwise, it looks
> > good to me.
>
> Hi, Dan
>
> Sorry for the late reply.
>
> I'm not sure if it's worth to add Fixes tags.
> These codes are very old which come from "Linux-2.6.12-rc2".
I know some people use Fixes tags to point to Linux-2.6.12-rc2 but
other people don't like it... Or they didn't like it back in the day,
I'm not sure now.
> It's seems like a cleanup or improvement.
It's definitely a Fix. It affects runtime.
>
> Umm, should I send v3 patches to add Fixes tags?
I don't really care, I guess. Probably yes? Not a lot of people use
aic7xxx these days so from a practical perspective it's not super
important either way.
regards,
dan carpenter
On 2023/12/5 16:14, Dan Carpenter wrote:
> On Tue, Dec 05, 2023 at 11:33:36AM +0800, Su Hui wrote:
>> On 2023/12/1 15:53, Dan Carpenter wrote:
>>> On Fri, Dec 01, 2023 at 10:59:53AM +0800, Su Hui wrote:
>>>> v2:
>>>> - fix some problems and split v1 patch into this patch set.(Thanks to
>>>> Dan)
>>>>
>>>> v1:
>>>> - https://lore.kernel.org/all/[email protected]/
>>>>
>>> Would have been better with Fixes tags probably. Otherwise, it looks
>>> good to me.
>> Hi, Dan
>>
>> Sorry for the late reply.
>>
>> I'm not sure if it's worth to add Fixes tags.
>> These codes are very old which come from "Linux-2.6.12-rc2".
> I know some people use Fixes tags to point to Linux-2.6.12-rc2 but
> other people don't like it... Or they didn't like it back in the day,
> I'm not sure now.
>
>> It's seems like a cleanup or improvement.
> It's definitely a Fix. It affects runtime.
>
>> Umm, should I send v3 patches to add Fixes tags?
> I don't really care, I guess. Probably yes? Not a lot of people use
> aic7xxx these days so from a practical perspective it's not super
> important either way.
Got it. Maybe it's right to omit these old Fixes tags.
Su Hui
Su,
> v2:
> - fix some problems and split v1 patch into this patch set.(Thanks to
> Dan)
Applied to 6.8/scsi-staging, thanks!
--
Martin K. Petersen Oracle Linux Engineering
On Fri, 01 Dec 2023 10:59:53 +0800, Su Hui wrote:
> v2:
> - fix some problems and split v1 patch into this patch set.(Thanks to
> Dan)
>
> v1:
> - https://lore.kernel.org/all/[email protected]/
>
> [...]
Applied to 6.8/scsi-queue, thanks!
[1/3] scsi: aic7xxx: return negative error codes in ahc_linux_register_host()
https://git.kernel.org/mkp/scsi/c/573eb4a3410a
[2/3] scsi: aic7xxx: return ahc_linux_register_host()'s value rather than zero
https://git.kernel.org/mkp/scsi/c/70dfaf84ec77
[3/3] scsi: aic7xxx: return negative error codes in aic7770_probe()
https://git.kernel.org/mkp/scsi/c/aef6ac123609
--
Martin K. Petersen Oracle Linux Engineering