Re: [PATCH v2 02/10] arm: socfpga: iossm: add version check

[Michael Tretter <m.tretter@pengutronix.de>]

On Fri, 10 Apr 2026 10:18:58 +0200, Ahmad Fatoum wrote:
> On 4/9/26 3:52 PM, Michael Tretter wrote:
> > There is a version 1 of the IOSSM, which is used if the board
> > configuration was created with Quartus 25.3.0 or later.
> > 
> > Warn the user if barebox didn't detect the supported version 0.
> > 
> > Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
> > ---
> > Changes in v2:
> > 
> > - none
> > ---
> >  arch/arm/mach-socfpga/iossm_mailbox.c | 27 +++++++++++++++++++++++++++
> >  arch/arm/mach-socfpga/iossm_mailbox.h |  2 ++
> >  2 files changed, 29 insertions(+)
> > 
> > diff --git a/arch/arm/mach-socfpga/iossm_mailbox.c b/arch/arm/mach-socfpga/iossm_mailbox.c
> > index d89117da4fd4..d14399f305de 100644
> > --- a/arch/arm/mach-socfpga/iossm_mailbox.c
> > +++ b/arch/arm/mach-socfpga/iossm_mailbox.c
> > @@ -27,6 +27,9 @@
> >  #define INTF_IP_TYPE_MASK	GENMASK(31, 29)
> >  #define INTF_INSTANCE_ID_MASK	GENMASK(28, 24)
> >  
> > +#define IOSSM_MAILBOX_HEADER_OFFSET			0x0
> > +#define IOSSM_MAILBOX_SPEC_VERSION_MASK			GENMASK(2, 0)
> > +
> >  /* supported DDR type list */
> >  static const char *ddr_type_list[7] = {
> >  		"DDR4", "DDR5", "DDR5_RDIMM", "LPDDR4", "LPDDR5", "QDRIV", "UNKNOWN"
> > @@ -163,6 +166,18 @@ int io96b_mb_req(phys_addr_t io96b_csr_addr, u32 ip_type, u32 instance_id,
> >  	return 0;
> >  }
> >  
> > +static int io96b_mb_version(struct io96b_info *io96b_ctrl)
> > +{
> > +	phys_addr_t io96b_csr_addr = io96b_ctrl->io96b[0].io96b_csr_addr;
> > +	u32 mailbox_header;
> > +	int version;
> > +
> > +	mailbox_header = readl(io96b_csr_addr + IOSSM_MAILBOX_HEADER_OFFSET);
> 
> Nitpick: readl() is supposed to take void __iomem * (virtual addresses),
> not physical addresses, so a phys_to_virt is missing here.
> 
> They currently expands to the same value (with different types) though
> on ARM.

The entire driver uses readl() with io96b_csr_addr as phys_addr_t
without calling phys_to_virt. There are occasionally sprinkled casts
using IOMEM(), but it's not consistent. It may be worth doing a cleanup
across the entire driver, but that should be a separate patch series.

Michael

> 
> You don't have to resend just for this though.
> 
> Thanks,
> Ahmad
> 
> > +	version = FIELD_GET(IOSSM_MAILBOX_SPEC_VERSION_MASK, mailbox_header);
> > +
> > +	return version;
> > +}
> > +
> >  /*
> >   * Initial function to be called to set memory interface IP type and instance ID
> >   * IP type and instance ID need to be determined before sending mailbox command
> > @@ -172,6 +187,18 @@ void io96b_mb_init(struct io96b_info *io96b_ctrl)
> >  	struct io96b_mb_resp usr_resp;
> >  	u8 ip_type_ret, instance_id_ret;
> >  	int i, j, k;
> > +	int version;
> > +
> > +	version = io96b_mb_version(io96b_ctrl);
> > +	switch (version) {
> > +	case 0:
> > +		pr_debug("IOSSM: mailbox version %d\n", version);
> > +		break;
> > +	default:
> > +		pr_warn("IOSSM: unsupported mailbox version %d\n", version);
> > +		break;
> > +	}
> > +	io96b_ctrl->version = version;
> >  
> >  	pr_debug("%s: num_instance %d\n", __func__, io96b_ctrl->num_instance);
> >  	for (i = 0; i < io96b_ctrl->num_instance; i++) {
> > diff --git a/arch/arm/mach-socfpga/iossm_mailbox.h b/arch/arm/mach-socfpga/iossm_mailbox.h
> > index 29b3f069072d..bd66621d5f70 100644
> > --- a/arch/arm/mach-socfpga/iossm_mailbox.h
> > +++ b/arch/arm/mach-socfpga/iossm_mailbox.h
> > @@ -110,6 +110,7 @@ struct io96b_instance {
> >  /*
> >   * Overall IO96B instance(s) information
> >   *
> > + * @version:		Version of the IO96B
> >   * @num_instance:	Number of instance(s) assigned to HPS
> >   * @overall_cal_status: Overall calibration status for all IO96B instance(s)
> >   * @ddr_type:		DDR memory type
> > @@ -120,6 +121,7 @@ struct io96b_instance {
> >   * @num_port:		Number of IO96B port.
> >   */
> >  struct io96b_info {
> > +	int			 version;
> >  	u8			 num_instance;
> >  	bool			 overall_cal_status;
> >  	const char		*ddr_type;
> >