Re: [PATCH v5 08/10] misc: rp1: RaspberryPi RP1 misc driver

Andrea della Porta
2025-01-09T14:12:55Z

Hi Rob,

On 15:08 Mon 16 Dec , Andrea della Porta wrote:
> Hi Rob,
>
> On 16:48 Tue 10 Dec , Rob Herring wrote:
> > On Mon, Dec 02, 2024 at 12:19:32PM +0100, Andrea della Porta wrote:
> > > The RaspberryPi RP1 is a PCI multi function device containing
> > > peripherals ranging from Ethernet to USB controller, I2C, SPI
> > > and others.

...

> > > +#define RP1_INT_ADC_FIFO 52
> > > +#define RP1_INT_PCIE_OUT 53
> > > +#define RP1_INT_SPI6 54
> > > +#define RP1_INT_SPI7 55
> > > +#define RP1_INT_SPI8 56
> > > +#define RP1_INT_SYSCFG 58
> > > +#define RP1_INT_CLOCKS_DEFAULT 59
> > > +#define RP1_INT_VBUSCTRL 60
> > > +#define RP1_INT_PROC_MISC 57
> >
> > Why all these defines which will never be used because they come from
> > DT?
> >
>
> Right, those defines where originally designed to be included from dts, but
> previous discussion deemed interrupt numbers to be hardcoded instead of being
> specified as mnemonics. In the driver source code I just use RP1_INT_END as the
> number of interrupts but I thought that the specific interrupt numbers should
> be documented in some way or another. Since no one is currently referencing
> those defines, would it be better to just turn those in a multiline comment
> just to describe them in a more compact form?

So, here's a couple of proposals about the interrupt defines:

- since they were banned from devicetree, and are not used anywhere in the code,
turn them into a (admittedly long) multiline comment, so they are still at
least documented

- since they were banned from devicetree, and are not use anywhere in the code,
just drop them, we don't currently need them after all

Not sure what's the best way here, anyone can advise?

Many thanks,
Andrea