Hi Florian, On Sun, 31 May 2020 at 00:18, Florian Fainelli <f.faine...@gmail.com> wrote: > > > > On 5/30/2020 4:51 AM, Vladimir Oltean wrote: > > From: Vladimir Oltean <vladimir.olt...@nxp.com> > > > > Currently Felix and Ocelot share the same bit layout in these per-port > > registers, but Seville does not. So we need reg_fields for that. > > > > Actually since these are per-port registers, we need to also specify the > > number of ports, and register size per port, and use the regmap API for > > multiple ports. > > > > Signed-off-by: Vladimir Oltean <vladimir.olt...@nxp.com> > > --- > > Changes in v2: > > None. > > [snip] > > > > /* Core: Enable port for frame transfer */ > > - ocelot_write_rix(ocelot, QSYS_SWITCH_PORT_MODE_INGRESS_DROP_MODE | > > - QSYS_SWITCH_PORT_MODE_SCH_NEXT_CFG(1) | > > - QSYS_SWITCH_PORT_MODE_PORT_ENA, > > - QSYS_SWITCH_PORT_MODE, port); > > + ocelot_fields_write(ocelot, port, > > + QSYS_SWITCH_PORT_MODE_INGRESS_DROP_MODE, 1); > > + ocelot_fields_write(ocelot, port, > > + QSYS_SWITCH_PORT_MODE_PORT_ENA, 1); > > I am a bit confused throughout this patch sometimes SCH_NEXT_CFG is set > to 1, sometimes not, this makes it a bit harder to review the > conversion, assuming this is fine: > > Reviewed-by: Florian Fainelli <f.faine...@gmail.com> > -- > Florian
Yes, this is a subtle point, but it's correct the way it is, and I didn't want to insist on the details of it, but now that you mentioned it, let's go. Seville does not have the QSYS_SWITCH_PORT_MODE_SCH_NEXT_CFG register field at all. And using the previous API (ocelot_write_rix), we were only writing 1 for Felix and Ocelot, which was their hardware-default value, so we weren't changing its value in practice. So the equivalent with ocelot_fields_write would be to simply not do anything at all for the SCH_NEXT_CFG field, which is actually something that is required if we want to support Seville too. Thank you so much for reviewing the entire series! -Vladimir
