On 12/14/23 2:45 PM, Claudiu wrote:
> From: Claudiu Beznea <claudiu.beznea.uj@xxxxxxxxxxxxxx>
>
> Move the IRQs get and request in the driver's probe function. As some IP
> variants switches to reset operation mode as a result of setting module
s/switches/switch/.
Also, the manuals call this "operating mode", not to mix with one of
the modes -- "operation mode".
> standby through clock enable/disable APIs, to implement runtime PM the
> resource parsing and requests are moved in the probe function and IP
Requesting.
Could you explain in more detail why you need to do this?
> settings are moved in the open functions. This is a preparatory change to
I don't see you moving anything into ravb_open() here...
> add runtime PM support for all IP variants.
>
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@xxxxxxxxxxxxxx>
[...]
> diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
> index 83691a0f0cc2..d7f6e8ea8e79 100644
> --- a/drivers/net/ethernet/renesas/ravb_main.c
> +++ b/drivers/net/ethernet/renesas/ravb_main.c
> @@ -1731,7 +1731,7 @@ static inline int ravb_hook_irq(unsigned int irq, irq_handler_t handler,
> name = devm_kasprintf(dev, GFP_KERNEL, "%s:%s", ndev->name, ch);
Ugh, I didn't realize we had the managed device API call in a function
called from ravb_open()... :-/
[...]
> @@ -2616,6 +2536,127 @@ static void ravb_parse_delay_mode(struct device_node *np, struct net_device *nde
> }
> }
>
> +static int ravb_get_irqs(struct ravb_private *priv)
> +{
> + const char *err_a_irq_name = NULL, *mgmt_a_irq_name = NULL;
You don't seem to use these as the pointers. Could be bool instead?
But even that doesn't seem necessary..
> + const struct ravb_hw_info *info = priv->info;
> + struct platform_device *pdev = priv->pdev;
> + struct net_device *ndev = priv->ndev;
> + const char *irq_name, *emac_irq_name;
> + int i, irq;
> +
> + if (!info->multi_irqs) {
> + irq = platform_get_irq(pdev, 0);
> + if (irq < 0)
> + return irq;
> +
> + ndev->irq = irq;
> + return 0;
> + }
> +
> + if (info->err_mgmt_irqs) {
> + irq_name = "dia";
> + emac_irq_name = "line3";
> + err_a_irq_name = "err_a";
> + mgmt_a_irq_name = "mgmt_a";
> + } else {
> + irq_name = "ch22";
> + emac_irq_name = "ch24";
> + }
> +
> + irq = platform_get_irq_byname(pdev, irq_name);
> + if (irq < 0)
> + return irq;
> + ndev->irq = irq;
> +
> + irq = platform_get_irq_byname(pdev, emac_irq_name);
> + if (irq < 0)
> + return irq;
> + priv->emac_irq = irq;
> +
> + if (err_a_irq_name) {
Why not just ctest info->err_mgmt_irqs here, as it was before
this patch?
> + irq = platform_get_irq_byname(pdev, "err_a");
> + if (irq < 0)
> + return irq;
> + priv->erra_irq = irq;
> + }
> +
> + if (mgmt_a_irq_name) {
> + irq = platform_get_irq_byname(pdev, "mgmt_a");
> + if (irq < 0)
> + return irq;
> + priv->mgmta_irq = irq;
> + }
> +
> + for (i = 0; i < NUM_RX_QUEUE; i++) {
> + irq = platform_get_irq_byname(pdev, ravb_rx_irqs[i]);
> + if (irq < 0)
> + return irq;
> + priv->rx_irqs[i] = irq;
> + }
> + for (i = 0; i < NUM_TX_QUEUE; i++) {
> + irq = platform_get_irq_byname(pdev, ravb_tx_irqs[i]);
> + if (irq < 0)
> + return irq;
> + priv->tx_irqs[i] = irq;
> + }
> +
> + return 0;
> +}
> +
> +static int ravb_request_irqs(struct ravb_private *priv)
I'm not sure separating getting and requesting IRQs is a good idea.
As you're switching to using the managed device API anyway, you could
save on some IRQ-related fields in the *struct* ravb_private, I think...
[...]
MBR, Sergey
