Re: [RFC PATCH net-next 01/14] net: mscc: ocelot: introduce a new ocelot_target_{read,write} API

[Alexandre Belloni <alexandre.belloni@bootlin.com>]

On 25/09/2020 15:18:42+0300, Vladimir Oltean wrote:
> There are some targets (register blocks) in the Ocelot switch that are
> instantiated more than once. For example, the VCAP IS1, IS2 and ES0
> blocks all share the same register layout for interacting with the cache
> for the TCAM and the action RAM.
> 
> For the VCAPs, the procedure for servicing them is actually common. We
> just need an API specifying which VCAP we are talking to, and we do that
> via these raw ocelot_target_read and ocelot_target_write accessors.
> 
> In plain ocelot_read, the target is encoded into the register enum
> itself:
> 
> 	u16 target = reg >> TARGET_OFFSET;
> 
> For the VCAPs, the registers are currently defined like this:
> 
> 	enum ocelot_reg {
> 	[...]
> 		S2_CORE_UPDATE_CTRL = S2 << TARGET_OFFSET,
> 		S2_CORE_MV_CFG,
> 		S2_CACHE_ENTRY_DAT,
> 		S2_CACHE_MASK_DAT,
> 		S2_CACHE_ACTION_DAT,
> 		S2_CACHE_CNT_DAT,
> 		S2_CACHE_TG_DAT,
> 	[...]
> 	};
> 
> which is precisely what we want to avoid, because we'd have to duplicate
> the same register map for S1 and for S0, and then figure out how to pass
> VCAP instance-specific registers to the ocelot_read calls (basically
> another lookup table that undoes the effect of shifting with
> TARGET_OFFSET).
> 
> So for some targets, propose a more raw API, similar to what is
> currently done with ocelot_port_readl and ocelot_port_writel. Those
> targets can only be accessed with ocelot_target_{read,write} and not
> with ocelot_{read,write} after the conversion, which is fine.
> 
> The VCAP registers are not actually modified to use this new API as of
> this patch. They will be modified in the next one.
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Acked-by: Alexandre Belloni <alexandre.belloni@bootlin.com>

> ---
>  drivers/net/ethernet/mscc/ocelot_io.c | 17 +++++++++++++++++
>  include/soc/mscc/ocelot.h             | 14 ++++++++++++++
>  2 files changed, 31 insertions(+)
> 
> diff --git a/drivers/net/ethernet/mscc/ocelot_io.c b/drivers/net/ethernet/mscc/ocelot_io.c
> index d22711282183..0acb45948418 100644
> --- a/drivers/net/ethernet/mscc/ocelot_io.c
> +++ b/drivers/net/ethernet/mscc/ocelot_io.c
> @@ -71,6 +71,23 @@ void ocelot_port_writel(struct ocelot_port *port, u32 val, u32 reg)
>  }
>  EXPORT_SYMBOL(ocelot_port_writel);
>  
> +u32 __ocelot_target_read_ix(struct ocelot *ocelot, enum ocelot_target target,
> +			    u32 reg, u32 offset)
> +{
> +	u32 val;
> +
> +	regmap_read(ocelot->targets[target],
> +		    ocelot->map[target][reg] + offset, &val);
> +	return val;
> +}
> +
> +void __ocelot_target_write_ix(struct ocelot *ocelot, enum ocelot_target target,
> +			      u32 val, u32 reg, u32 offset)
> +{
> +	regmap_write(ocelot->targets[target],
> +		     ocelot->map[target][reg] + offset, val);
> +}
> +
>  int ocelot_regfields_init(struct ocelot *ocelot,
>  			  const struct reg_field *const regfields)
>  {
> diff --git a/include/soc/mscc/ocelot.h b/include/soc/mscc/ocelot.h
> index 7e52e6ee09d8..a71ea217da70 100644
> --- a/include/soc/mscc/ocelot.h
> +++ b/include/soc/mscc/ocelot.h
> @@ -662,6 +662,16 @@ struct ocelot_policer {
>  #define ocelot_fields_write(ocelot, id, reg, val) regmap_fields_write((ocelot)->regfields[(reg)], (id), (val))
>  #define ocelot_fields_read(ocelot, id, reg, val) regmap_fields_read((ocelot)->regfields[(reg)], (id), (val))
>  
> +#define ocelot_target_read_ix(ocelot, target, reg, gi, ri) __ocelot_target_read_ix(ocelot, target, reg, reg##_GSZ * (gi) + reg##_RSZ * (ri))
> +#define ocelot_target_read_gix(ocelot, target, reg, gi) __ocelot_target_read_ix(ocelot, target, reg, reg##_GSZ * (gi))
> +#define ocelot_target_read_rix(ocelot, target, reg, ri) __ocelot_target_read_ix(ocelot, target, reg, reg##_RSZ * (ri))
> +#define ocelot_target_read(ocelot, target, reg) __ocelot_target_read_ix(ocelot, target, reg, 0)
> +
> +#define ocelot_target_write_ix(ocelot, target, val, reg, gi, ri) __ocelot_target_write_ix(ocelot, target, val, reg, reg##_GSZ * (gi) + reg##_RSZ * (ri))
> +#define ocelot_target_write_gix(ocelot, target, val, reg, gi) __ocelot_target_write_ix(ocelot, target, val, reg, reg##_GSZ * (gi))
> +#define ocelot_target_write_rix(ocelot, target, val, reg, ri) __ocelot_target_write_ix(ocelot, target, val, reg, reg##_RSZ * (ri))
> +#define ocelot_target_write(ocelot, target, val, reg) __ocelot_target_write_ix(ocelot, target, val, reg, 0)
> +
>  /* I/O */
>  u32 ocelot_port_readl(struct ocelot_port *port, u32 reg);
>  void ocelot_port_writel(struct ocelot_port *port, u32 val, u32 reg);
> @@ -669,6 +679,10 @@ u32 __ocelot_read_ix(struct ocelot *ocelot, u32 reg, u32 offset);
>  void __ocelot_write_ix(struct ocelot *ocelot, u32 val, u32 reg, u32 offset);
>  void __ocelot_rmw_ix(struct ocelot *ocelot, u32 val, u32 mask, u32 reg,
>  		     u32 offset);
> +u32 __ocelot_target_read_ix(struct ocelot *ocelot, enum ocelot_target target,
> +			    u32 reg, u32 offset);
> +void __ocelot_target_write_ix(struct ocelot *ocelot, enum ocelot_target target,
> +			      u32 val, u32 reg, u32 offset);
>  
>  /* Hardware initialization */
>  int ocelot_regfields_init(struct ocelot *ocelot,
> -- 
> 2.25.1
> 

-- 
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com