Re: [PATCH 2/2] gpio: rcar: Add R-Car X5H (R8A78000) support

[Marek Vasut]

On 7/6/26 11:19 AM, Bartosz Golaszewski wrote:

Hello Bartosz,

> > +static inline int gpio_rcar_remap_offset(struct gpio_rcar_priv *p, int *offs)
> > +{

I am hoping to get some input on this remap function.

> > +	/* 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);
> > +		return -EINVAL;
> > +	}
> > +}
> > +

[...]

> > @@ -399,6 +445,7 @@ static const struct gpio_rcar_info gpio_rcar_info_gen1 = {
> >   	.has_both_edge_trigger = false,
> >   	.has_always_in = false,
> >   	.has_inen = false,
> > +	.has_layout_gen5 = false,
> >   };
> >
> >   static const struct gpio_rcar_info gpio_rcar_info_gen2 = {
> > @@ -406,6 +453,7 @@ static const struct gpio_rcar_info gpio_rcar_info_gen2 = {
> >   	.has_both_edge_trigger = true,
> >   	.has_always_in = false,
> >   	.has_inen = false,
> > +	.has_layout_gen5 = false,
> >   };
> >
> >   static const struct gpio_rcar_info gpio_rcar_info_gen3 = {
> > @@ -413,6 +461,7 @@ static const struct gpio_rcar_info gpio_rcar_info_gen3 = {
> >   	.has_both_edge_trigger = true,
> >   	.has_always_in = true,
> >   	.has_inen = false,
> > +	.has_layout_gen5 = false,
> >   };
> >
> >   static const struct gpio_rcar_info gpio_rcar_info_gen4 = {
> > @@ -420,6 +469,15 @@ static const struct gpio_rcar_info gpio_rcar_info_gen4 = {
> >   	.has_both_edge_trigger = true,
> >   	.has_always_in = true,
> >   	.has_inen = true,
> > +	.has_layout_gen5 = false,
>
> This looks good but do we really need to change these lines if it's zeroes
> anyway?

I'm just following the pattern in the driver, which does full assignment 
of all fields for all generations of the controller. See .has_inen = 
false in Gen1 for example. We don't have to do the assignment, but maybe 
doing so is more comprehensible ?

--
Best regards,
Marek Vasut