From: Claudiu Beznea <[email protected]>
Hi,
Series adds one fix for the ravb driver to wait for the operating
mode to be applied by hardware before proceeding.
Thank you,
Claudiu Beznea
Changes in v3:
- s/csr_opmode/csr_ops in ravb_set_opmode()
- mask opmode with CCC_OPC when retrieveing csr_ops
- in ravb_set_opmode() updated modify mask passed to ravb_modify() to
include GAC and CSEL bits if these are part of opmode
- s/failed to switch device to config mode/failed to switch device to
requested mode/ in ravb_set_opmode()
- s/operation/operating in the title of patch 1/1
Changes in v2:
- dropped patch 2/2 from v1 ("net: ravb: Check that GTI loading request is
done")
- kept a single "Fixes" entry in commit description
- updated commit description for patch 1/1
- introduce ravb_set_opmode() that does all the necessities for
setting the operating mode (set DMA.CCC and wait for CSR.OPS) and call it
where needed; this should comply with all the HW manuals requirements as
different manual variants specify different modes need to be checked in
CSR.OPS when setting DMA.CCC.
Claudiu Beznea (1):
net: ravb: Wait for operation mode to be applied
drivers/net/ethernet/renesas/ravb_main.c | 61 +++++++++++++++---------
1 file changed, 38 insertions(+), 23 deletions(-)
--
2.39.2
From: Claudiu Beznea <[email protected]>
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
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 <[email protected]>
---
drivers/net/ethernet/renesas/ravb_main.c | 61 +++++++++++++++---------
1 file changed, 38 insertions(+), 23 deletions(-)
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)
+ ccc_mask |= CCC_GAC | CCC_CSEL;
+
+ /* Set operating mode */
+ ravb_modify(ndev, CCC, ccc_mask, opmode);
+ /* Check if the operating mode is changed to the requested one */
+ error = ravb_wait(ndev, CSR, CSR_OPS, csr_ops);
+ if (error) {
+ netdev_err(ndev, "failed to switch device to requested mode (%u)\n",
+ opmode & CCC_OPC);
+ }
return error;
}
@@ -673,7 +680,7 @@ static int ravb_dmac_init(struct net_device *ndev)
int error;
/* Set CONFIG mode */
- error = ravb_config(ndev);
+ error = ravb_set_opmode(ndev, CCC_OPC_CONFIG);
if (error)
return error;
@@ -682,9 +689,7 @@ static int ravb_dmac_init(struct net_device *ndev)
return error;
/* Setting the control will start the AVB-DMAC process. */
- ravb_modify(ndev, CCC, CCC_OPC, CCC_OPC_OPERATION);
-
- return 0;
+ return ravb_set_opmode(ndev, CCC_OPC_OPERATION);
}
static void ravb_get_tx_tstamp(struct net_device *ndev)
@@ -1046,7 +1051,7 @@ static int ravb_stop_dma(struct net_device *ndev)
return error;
/* Stop AVB-DMAC process */
- return ravb_config(ndev);
+ return ravb_set_opmode(ndev, CCC_OPC_CONFIG);
}
/* E-MAC interrupt handler */
@@ -2560,21 +2565,25 @@ static int ravb_set_gti(struct net_device *ndev)
return 0;
}
-static void ravb_set_config_mode(struct net_device *ndev)
+static int ravb_set_config_mode(struct net_device *ndev)
{
struct ravb_private *priv = netdev_priv(ndev);
const struct ravb_hw_info *info = priv->info;
+ int error;
if (info->gptp) {
- ravb_modify(ndev, CCC, CCC_OPC, CCC_OPC_CONFIG);
+ error = ravb_set_opmode(ndev, CCC_OPC_CONFIG);
+ if (error)
+ return error;
/* Set CSEL value */
ravb_modify(ndev, CCC, CCC_CSEL, CCC_CSEL_HPB);
} else if (info->ccc_gac) {
- ravb_modify(ndev, CCC, CCC_OPC, CCC_OPC_CONFIG |
- CCC_GAC | CCC_CSEL_HPB);
+ error = ravb_set_opmode(ndev, CCC_OPC_CONFIG | CCC_GAC | CCC_CSEL_HPB);
} else {
- ravb_modify(ndev, CCC, CCC_OPC, CCC_OPC_CONFIG);
+ error = ravb_set_opmode(ndev, CCC_OPC_CONFIG);
}
+
+ return error;
}
/* Set tx and rx clock internal delay modes */
@@ -2794,7 +2803,9 @@ static int ravb_probe(struct platform_device *pdev)
ndev->ethtool_ops = &ravb_ethtool_ops;
/* Set AVB config mode */
- ravb_set_config_mode(ndev);
+ error = ravb_set_config_mode(ndev);
+ if (error)
+ goto out_disable_gptp_clk;
if (info->gptp || info->ccc_gac) {
/* Set GTI value */
@@ -2917,8 +2928,7 @@ static void ravb_remove(struct platform_device *pdev)
dma_free_coherent(ndev->dev.parent, priv->desc_bat_size, priv->desc_bat,
priv->desc_bat_dma);
- /* Set reset mode */
- ravb_write(ndev, CCC_OPC_RESET, CCC);
+ ravb_set_opmode(ndev, CCC_OPC_RESET);
clk_disable_unprepare(priv->gptp_clk);
clk_disable_unprepare(priv->refclk);
@@ -3000,8 +3010,11 @@ static int __maybe_unused ravb_resume(struct device *dev)
int ret = 0;
/* If WoL is enabled set reset mode to rearm the WoL logic */
- if (priv->wol_enabled)
- ravb_write(ndev, CCC_OPC_RESET, CCC);
+ if (priv->wol_enabled) {
+ ret = ravb_set_opmode(ndev, CCC_OPC_RESET);
+ if (ret)
+ return ret;
+ }
/* All register have been reset to default values.
* Restore all registers which where setup at probe time and
@@ -3009,7 +3022,9 @@ static int __maybe_unused ravb_resume(struct device *dev)
*/
/* Set AVB config mode */
- ravb_set_config_mode(ndev);
+ ret = ravb_set_config_mode(ndev);
+ if (ret)
+ return ret;
if (info->gptp || info->ccc_gac) {
/* Set GTI value */
--
2.39.2
On 1/2/24 2:01 PM, Claudiu wrote:
> From: Claudiu Beznea <[email protected]>
>
> 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 <[email protected]>
Reviewed-by: Sergey Shtylyov <[email protected]>
[...]
> 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