| Message ID | 20200925161929.1136806-8-tasleson@redhat.com |
|---|---|
| State | Not Applicable |
| Delegated to: | David Miller |
| Headers | show |
| Series | Add persistent durable identifier to storage log messages | expand |
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
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?
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
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 --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);