Re: [PATCH 2/2] gpio: rcar: Add R-Car X5H (R8A78000) support
From: Marek Vasut
Date: Sat Sep 05 2026 - 17:58:14 EST
On 9/4/26 1:15 PM, Geert Uytterhoeven wrote:
Hello Geert,
+++ b/drivers/gpio/gpio-rcar.c
@@ -65,14 +66,59 @@ struct gpio_rcar_priv {
#define RCAR_MAX_GPIO_PER_BANK 32
+static inline int gpio_rcar_remap_offset(struct gpio_rcar_priv *p, int *offs)
IMO passing a pointer to offs complicates the code. Perhaps pass offs
by value, and return the adjusted offset or a negative error code?
I want to avoid that, since if I only return error value, I can then do simple:
ret = gpio_rcar_remap_offset(...);
if (ret)
return ret;
in gpio_rcar_read() and gpio_rcar_write(), which are the only two call sites of this function.
I had one more idea -- how about we convert the driver to mmio regmap, use opaque register numbers throughout the driver to identify registers to the regmap (maybe not a great idea), and then implement .read/.write callbacks in the regmap_config which instead of doing plain readl()/writel() for register IO would instead do this remapping ? Regmap could validate that the opaque register numbers are only the expected register numbers and reject all the others. Maybe the opaque register numbers could instead of Gen4 register offsets. What do you think ?+{
+ /* R-Car Gen4 and older do not need any offset remap. */
+ if (!p->info.has_layout_gen5)
+ return 0;
+
+ /*
+ * R-Car Gen5 register layout is slightly different and the offsets
+ * that have to be added to or subtracted from each register offset
+ * can be divided into five groups, listed below.
+ */
+ switch (*offs) {
+ case IOINTSEL...OUTDT:
+ return 0;
+ case INDT:
+ *offs += 0x10;
+ return 0;
+ case INTDT...EDGLEVEL:
+ fallthrough;
+ case BOTHEDGE:
+ *offs += 0x70;
+ return 0;
+ case OUTDTSEL:
+ *offs -= 0x34;
+ return 0;
+ case INEN:
+ *offs -= 0x38;
+ return 0;
+ default:
+ /*
+ * This here must never be reached, if this is reached, that
+ * means there is a catastrophic failure in the driver. Skip
+ * any IO read/write to prevent further damage.
+ */
+ WARN_ON(1);
A build-time failure would be better. I tried BUILD_BUG() instead,
but unfortunately gcc is not smart enough to notice this case is
never reached. __always_inline doesn't seem to help either.