Re: [PATCH v6 1/2] Documentation: bindings: add dt doc for Rockchip PCIe controller

From: Brian Norris <briannorris-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
To: Shawn Lin <shawn.lin-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Heiko Stuebner <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org>,
	Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>,
	Marc Zyngier <marc.zyngier-5wv7dgnIgG8@public.gmane.org>,
	linux-pci-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Wenrui Li <wenrui.li-TNX95d0MmH7DzftRWevZcw@public.gmane.org>,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Doug Anderson <dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>,
	linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
	Bjorn Helgaas <bhelgaas-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
Subject: Re: [PATCH v6 1/2] Documentation: bindings: add dt doc for Rockchip PCIe controller
Date: Tue, 12 Jul 2016 19:05:31 -0700	[thread overview]
Message-ID: <20160713020531.GA14833@google.com> (raw)
In-Reply-To: <95a7ffdd-becc-bb3f-c026-d07c691b7a53-TNX95d0MmH7DzftRWevZcw@public.gmane.org>

Hi,

On Wed, Jul 13, 2016 at 09:45:43AM +0800, Shawn Lin wrote:
> 在 2016/7/13 9:31, Brian Norris 写道:
> >On Wed, Jul 13, 2016 at 09:10:15AM +0800, Shawn Lin wrote:
> >At some level, it's a matter of preference. But when you're talking
> >about the rk3399 PCIe "interrupt controller" domain, it seems that you
> >should be talking about HW bits in the controller -- i.e., you have a
> >4-bit interrupt status bitfield, that we typically call [0:3]. If you
> >use [1:4], then you have to remember to subtract 1 mentally when mapping
> >to the actual HW bit. I believe that confusion (since bitfields normally
> >count from 0) might have helped cause the infinite loop bug I noticed
> >too. And I also think that counting from 0 helps clarify the fact that
> >your interrupt controller indexing is an independent numbering from the
> >PCI interrupt numbering, even though they happen to map 1:1.
> 
> If that's the fact of how we should numbering our index base, we should
> probably start if from 5 as the layout of INTx is
> PCIE_CLIENT_INT_STATUS[5:8]... ?

Possibly better than starting from 1, but IMO also doesn't make sense,
because the other bits aren't interrupts you want to translate on behalf
of other devices (are they?) -- they're interrupt bits consumed by the
host controller itself. (If they are possibly needed for translation,
then sure, index the entire status register and handle it in the driver,
and start the INTx mapping from 5 here.)

[...]

> >If you still think it makes more sense to count from 1, then I won't
> >stop you.
> 
> I don't have a hard opinion for the index base as I think it's trivial.

It's simple, but I think it influenced code understanding and bugginess.

> So if it's more sensible to you, I will apply your suggestion.

Well, I was just offering my opinion. I think it makes more sense, but
maybe it doesn't to you.

Brian

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip