On 1/2/24 2:01 PM, Claudiu wrote:
> From: Claudiu Beznea <claudiu.beznea.uj@xxxxxxxxxxxxxx>
>
> CSR.OPS bits specify the current operating mode and (according to
> documentation) they are updated by HW when the operating mode change
> request is processed. To comply with this check CSR.OPS before proceeding.
>
> Commit introduces ravb_set_opmode() that does all the necessities for
> setting the operating mode (set CCC.OPC (and CCC.GAC, CCC.CSEL, if any) and
> wait for CSR.OPS) and call it where needed. This should comply with all the
> HW manuals requirements as different manual variants specify that different
> modes need to be checked in CSR.OPS when setting CCC.OPC.
>
> In case of platforms with GAC, if GAC needs to be enabled, the CCC.GAC and
Better to spell it out, I think: in case of platforms that support gPTP
while in the config[uration] mode..
> CCC.CSEL needs to be configured along with CCC.OPC. For this,
> ravb_set_opmode() allows passing GAC and CSEL as part of opmode and the
> function updates accordingly CCC register.
>
> Fixes: c156633f1353 ("Renesas Ethernet AVB driver proper")
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@xxxxxxxxxxxxxx>
Reviewed-by: Sergey Shtylyov <s.shtylyov@xxxxxx>
[...]
> diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c> index 664eda4b5a11..9835d18a7adf 100644
> --- a/drivers/net/ethernet/renesas/ravb_main.c
> +++ b/drivers/net/ethernet/renesas/ravb_main.c
> @@ -66,16 +66,23 @@ int ravb_wait(struct net_device *ndev, enum ravb_reg reg, u32 mask, u32 value)
> return -ETIMEDOUT;
> }
>
> -static int ravb_config(struct net_device *ndev)
> +static int ravb_set_opmode(struct net_device *ndev, u32 opmode)
> {
> + u32 csr_ops = 1U << (opmode & CCC_OPC);
> + u32 ccc_mask = CCC_OPC;
> int error;
>
> - /* Set config mode */
> - ravb_modify(ndev, CCC, CCC_OPC, CCC_OPC_CONFIG);
> - /* Check if the operating mode is changed to the config mode */
> - error = ravb_wait(ndev, CSR, CSR_OPS, CSR_OPS_CONFIG);
> - if (error)
> - netdev_err(ndev, "failed to switch device to config mode\n");
> + if (opmode & CCC_GAC)
Worth a comment?
> + ccc_mask |= CCC_GAC | CCC_CSEL;
Adding CCC.GAC to the mask not strictly necessary but OK...
[...]
MBR, Sergey
