diff mbox series

[v5,07/12] libata: Make ata_scsi_durable_name static

Message ID 20200925161929.1136806-8-tasleson@redhat.com
State New
Headers show
Series Add persistent durable identifier to storage log messages | expand

Commit Message

Tony Asleson Sept. 25, 2020, 4:19 p.m. UTC
Signed-off-by: Tony Asleson <tasleson@redhat.com>
Signed-off-by: kernel test robot <lkp@intel.com>
---
 drivers/ata/libata-scsi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Sergei Shtylyov Sept. 26, 2020, 8:40 a.m. UTC | #1
Hello!

On 25.09.2020 19:19, Tony Asleson wrote:

> Signed-off-by: Tony Asleson <tasleson@redhat.com>
> Signed-off-by: kernel test robot <lkp@intel.com>
> ---
>   drivers/ata/libata-scsi.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
> index 194dac7dbdca..13a58ed7184c 100644
> --- a/drivers/ata/libata-scsi.c
> +++ b/drivers/ata/libata-scsi.c
> @@ -1086,7 +1086,7 @@ int ata_scsi_dev_config(struct scsi_device *sdev, struct ata_device *dev)
>   	return 0;
>   }
>   
> -int ata_scsi_durable_name(const struct device *dev, char *buf, size_t len)
> +static int ata_scsi_durable_name(const struct device *dev, char *buf, size_t len)

    Why not do it in patch #6 -- when introducing the function?

[...]

MBR, Sergei
Tony Asleson Sept. 26, 2020, 2:17 p.m. UTC | #2
On 9/26/20 3:40 AM, Sergei Shtylyov wrote:
> Hello!
> 
> On 25.09.2020 19:19, Tony Asleson wrote:
> 
>> Signed-off-by: Tony Asleson <tasleson@redhat.com>
>> Signed-off-by: kernel test robot <lkp@intel.com>
>> ---
>>   drivers/ata/libata-scsi.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
>> index 194dac7dbdca..13a58ed7184c 100644
>> --- a/drivers/ata/libata-scsi.c
>> +++ b/drivers/ata/libata-scsi.c
>> @@ -1086,7 +1086,7 @@ int ata_scsi_dev_config(struct scsi_device
>> *sdev, struct ata_device *dev)
>>       return 0;
>>   }
>>   -int ata_scsi_durable_name(const struct device *dev, char *buf,
>> size_t len)
>> +static int ata_scsi_durable_name(const struct device *dev, char *buf,
>> size_t len)
> 
>    Why not do it in patch #6 -- when introducing the function?

This issue was found by the intel kernel test robot in v4 patch series.
I thought it was better to have a separate commit with the correction
that matched it's signed off.  Maybe that's not the correct approach?
James Bottomley Sept. 26, 2020, 3:57 p.m. UTC | #3
On Sat, 2020-09-26 at 09:17 -0500, Tony Asleson wrote:
> On 9/26/20 3:40 AM, Sergei Shtylyov wrote:

> > Hello!

> > 

> > On 25.09.2020 19:19, Tony Asleson wrote:

> > 

> > > Signed-off-by: Tony Asleson <tasleson@redhat.com>

> > > Signed-off-by: kernel test robot <lkp@intel.com>

> > > ---

> > >   drivers/ata/libata-scsi.c | 2 +-

> > >   1 file changed, 1 insertion(+), 1 deletion(-)

> > > 

> > > diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-

> > > scsi.c

> > > index 194dac7dbdca..13a58ed7184c 100644

> > > --- a/drivers/ata/libata-scsi.c

> > > +++ b/drivers/ata/libata-scsi.c

> > > @@ -1086,7 +1086,7 @@ int ata_scsi_dev_config(struct scsi_device

> > > *sdev, struct ata_device *dev)

> > >       return 0;

> > >   }

> > >   -int ata_scsi_durable_name(const struct device *dev, char *buf,

> > > size_t len)

> > > +static int ata_scsi_durable_name(const struct device *dev, char

> > > *buf,

> > > size_t len)

> > 

> >    Why not do it in patch #6 -- when introducing the function?

> 

> This issue was found by the intel kernel test robot in v4 patch

> series. I thought it was better to have a separate commit with the

> correction that matched it's signed off.  Maybe that's not the

> correct approach?


No ... while a patch is being reviewed the purpose of review is to make
it better by folding in all the comments.  It then gets a changelog put
below the 

---

v5: made X function static

So people can follow how it has evolved.  This is all actually
described in Documentation/submitting-patches.

James
Tony Asleson Sept. 28, 2020, 8:35 p.m. UTC | #4
On 9/26/20 3:40 AM, Sergei Shtylyov wrote:
> 

>    Why not do it in patch #6 -- when introducing the function?


Re-working patch series, will do as you suggest and as outlined
in the submitting-patches.rst referenced by James.

Thanks
diff mbox series

Patch

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 194dac7dbdca..13a58ed7184c 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -1086,7 +1086,7 @@  int ata_scsi_dev_config(struct scsi_device *sdev, struct ata_device *dev)
 	return 0;
 }
 
-int ata_scsi_durable_name(const struct device *dev, char *buf, size_t len)
+static int ata_scsi_durable_name(const struct device *dev, char *buf, size_t len)
 {
 	struct ata_device *ata_dev = container_of(dev, struct ata_device, tdev);