public inbox for linux-pci@vger.kernel.org
 help / color / mirror / Atom feed
From: "Krzysztof Wilczyński" <kw@linux.com>
To: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Cc: lorenzo.pieralisi@arm.com, bhelgaas@google.com,
	svarbanov@mm-sol.com, bjorn.andersson@linaro.org,
	robh@kernel.org, linux-pci@vger.kernel.org,
	linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org,
	kernel test robot <lkp@intel.com>
Subject: Re: [PATCH v2] PCI: qcom: Use __be16 for catching cpu_to_be16() return instead of u16
Date: Tue, 30 Nov 2021 09:12:14 +0100	[thread overview]
Message-ID: <YaXc3mlBbhvEkZCX@rocinante> (raw)
In-Reply-To: <20211130080456.GE205712@thinkpad>

Hello!

[...]
> > > cpu_to_be16() returns __be16 value but the driver uses u16 and that's
> > > incorrect. Fix it by using __be16 as the datatype of bdf_be variable.
> > 
> > It would be "data type" in the above.
> > 
> > Not really a requirement to do so, but you could include the actual
> > warning, as sometimes this is useful for reference later, as per:
> > 
> >   drivers/pci/controller/dwc/pcie-qcom.c:1346:30: warning: incorrect type in initializer (different base types)
> >   drivers/pci/controller/dwc/pcie-qcom.c:1346:30:    expected unsigned short [usertype] bdf_be
> >   drivers/pci/controller/dwc/pcie-qcom.c:1346:30:    got restricted __be16 [usertype]
> > 
> 
> I usually do but as per Bjorn's comment I thought it is not recommended for PCI
> subsystem (or maybe I misread his comments). Will add.

Ah right.  I must have missed his comment too.  I usually include warnings
myself, where applicable.  Let's wait for what Bjorn says, just in case, so
that we avoid adding something he does not want to have included in the
commit message.

[...]
> > Also, since I have your attention, it seems we have a number of unused
> > macros in the qcom driver, as per:
> > 
> >   drivers/pci/controller/dwc/pcie-qcom.c:#define PCIE20_PARF_BDF_TRANSLATE_CFG            0x24C
> >   drivers/pci/controller/dwc/pcie-qcom.c:#define PCIE20_PARF_SID_OFFSET                   0x234
> >   drivers/pci/controller/dwc/pcie-qcom.c:#define PCIE20_PARF_SLV_ADDR_SPACE_SIZE          0x16C
> > 
> > And also in the qcom-ep driver, as per:
> > 
> 
> These defines are helpful for someone who wants to add some functionality or
> even bug fix in the future. Since these controllers are not publically
> documented, having these definitions helps a lot.

Got it!  I run a nightly CI pipeline and been seeing complains due to the
"-Wunused-macros" show up in the log, so I decided to steal the spotlight,
so to speak, to ask about these.

Thank you for letting me know.  Appreciated!

	Krzysztof

  reply	other threads:[~2021-11-30  8:12 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-30  6:42 [PATCH v2] PCI: qcom: Use __be16 for catching cpu_to_be16() return instead of u16 Manivannan Sadhasivam
2021-11-30  7:28 ` Krzysztof Wilczyński
2021-11-30  8:04   ` Manivannan Sadhasivam
2021-11-30  8:12     ` Krzysztof Wilczyński [this message]
2021-11-30  8:13       ` Krzysztof Wilczyński
2021-11-30 15:43       ` Bjorn Helgaas

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=YaXc3mlBbhvEkZCX@rocinante \
    --to=kw@linux.com \
    --cc=bhelgaas@google.com \
    --cc=bjorn.andersson@linaro.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=lkp@intel.com \
    --cc=lorenzo.pieralisi@arm.com \
    --cc=manivannan.sadhasivam@linaro.org \
    --cc=robh@kernel.org \
    --cc=svarbanov@mm-sol.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