Linux-ide Archive mirror
 help / color / mirror / Atom feed
From: Igor Pylypiv <ipylypiv@google.com>
To: John Garry <john.g.garry@oracle.com>
Cc: Damien Le Moal <dlemoal@kernel.org>,
	Niklas Cassel <cassel@kernel.org>,
	Jason Yan <yanaijie@huawei.com>,
	"James E.J. Bottomley" <jejb@linux.ibm.com>,
	"Martin K. Petersen" <martin.petersen@oracle.com>,
	Jack Wang <jinpu.wang@cloud.ionos.com>,
	Hannes Reinecke <hare@suse.de>,
	Xiang Chen <chenxiang66@hisilicon.com>,
	Artur Paszkiewicz <artur.paszkiewicz@intel.com>,
	Bart Van Assche <bvanassche@acm.org>,
	TJ Adams <tadamsjr@google.com>,
	linux-ide@vger.kernel.org, linux-scsi@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v5 2/7] scsi: libsas: Define NCQ Priority sysfs attributes for SATA devices
Date: Tue, 5 Mar 2024 15:58:48 -0800	[thread overview]
Message-ID: <ZeexuDYlmaDoDmtv@google.com> (raw)
In-Reply-To: <3fb62749-8c66-47ae-9b8b-7e670ebf4841@oracle.com>

On Tue, Mar 05, 2024 at 11:29:11AM +0000, John Garry wrote:
> On 05/03/2024 00:50, Igor Pylypiv wrote:
> >   static inline void sas_ata_disabled_notice(void)
> > @@ -123,6 +125,10 @@ static inline int sas_ata_add_dev(struct domain_device *parent, struct ex_phy *p
> >   	sas_ata_disabled_notice();
> >   	return -ENODEV;
> >   }
> > +
> > +static const struct attribute_group sas_ata_sdev_attr_group = {
> > +	.attrs = NULL,
> > +};
> 
> I just noticed a build issue.
> 
> With CONFIG_SCSI_SAS_ATA not set, I get this for W=1 build:
> 
> In file included from drivers/scsi/hisi_sas/hisi_sas.h:29,
>                 from drivers/scsi/hisi_sas/hisi_sas_v1_hw.c:7:
> ./include/scsi/sas_ata.h:129:37: error: ‘sas_ata_sdev_attr_group’
> defined but not used [-Werror=unused-const-variable=]
>  129 | static const struct attribute_group sas_ata_sdev_attr_group = {

Thanks for catching this, John!
For some reason I only get this warning with gcc but not with clang.

> 
> I suppose that marking sas_ata_sdev_attr_group as __maybe_unused is ok, but
> less than ideal. The linker should strip it out of files when unused.

Looks like adding the __maybe_unused attribute is a prefferred way since
it is mentioned in the Linux kernel coding style:
https://www.kernel.org/doc/html/v6.7/process/coding-style.html#conditional-compilation

Added the __maybe_unused attribute in v6. Thank you!

> 
> I think that this is also ok:
> 
> #define sas_ata_sdev_attr_group (struct attribute_group) {}
> 
> The compiler here will create a empty structure and have
> &sas_ata_sdev_attr_group point at it.
> 
> Thanks,
> John

  reply	other threads:[~2024-03-05 23:58 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-03-05  0:50 [PATCH v5 0/7] NCQ Priority sysfs sttributes for libsas Igor Pylypiv
2024-03-05  0:50 ` [PATCH v5 1/7] ata: libata-sata: Factor out NCQ Priority configuration helpers Igor Pylypiv
2024-03-05  2:44   ` Jason Yan
2024-03-05  0:50 ` [PATCH v5 2/7] scsi: libsas: Define NCQ Priority sysfs attributes for SATA devices Igor Pylypiv
2024-03-05  2:44   ` Jason Yan
2024-03-05 11:29   ` John Garry
2024-03-05 23:58     ` Igor Pylypiv [this message]
2024-03-05  0:50 ` [PATCH v5 3/7] scsi: pm80xx: Add libsas SATA sysfs attributes group Igor Pylypiv
2024-03-05  2:45   ` Jason Yan
2024-03-05  0:51 ` [PATCH v5 4/7] scsi: mvsas: " Igor Pylypiv
2024-03-05  2:45   ` Jason Yan
2024-03-05  0:51 ` [PATCH v5 5/7] scsi: hisi_sas: " Igor Pylypiv
2024-03-05  2:46   ` Jason Yan
2024-03-05  0:51 ` [PATCH v5 6/7] scsi: aic94xx: " Igor Pylypiv
2024-03-05  2:46   ` Jason Yan
2024-03-05  0:51 ` [PATCH v5 7/7] scsi: isci: " Igor Pylypiv
2024-03-05  2:47   ` Jason Yan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZeexuDYlmaDoDmtv@google.com \
    --to=ipylypiv@google.com \
    --cc=artur.paszkiewicz@intel.com \
    --cc=bvanassche@acm.org \
    --cc=cassel@kernel.org \
    --cc=chenxiang66@hisilicon.com \
    --cc=dlemoal@kernel.org \
    --cc=hare@suse.de \
    --cc=jejb@linux.ibm.com \
    --cc=jinpu.wang@cloud.ionos.com \
    --cc=john.g.garry@oracle.com \
    --cc=linux-ide@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=martin.petersen@oracle.com \
    --cc=tadamsjr@google.com \
    --cc=yanaijie@huawei.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).