* [PATCH 0/6] R-Can CAN FD driver enhancements
@ 2022-10-22 10:43 Biju Das
2022-10-22 10:43 ` [PATCH 1/6] can: rcar_canfd: rcar_canfd_probe: Add struct rcar_canfd_hw_info to driver data Biju Das
` (5 more replies)
0 siblings, 6 replies; 18+ messages in thread
From: Biju Das @ 2022-10-22 10:43 UTC (permalink / raw)
To: Wolfgang Grandegger, Marc Kleine-Budde, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni
Cc: Biju Das, Vincent Mailhol, Stefan Mätje, Ulrich Hecht,
Lad Prabhakar, Christophe JAILLET, linux-can, netdev,
Geert Uytterhoeven, Chris Paterson, Biju Das, linux-renesas-soc
The CAN FD IP found on RZ/G2L SoC has some HW features different to that
of R-Car. For example, it has multiple resets and multiple IRQs for global
and channel interrupts. Also, it does not have ECC error flag registers
and clk post divider present on R-Car. Similarly, R-Car V3U has 8 channels
whereas other SoCs has only 2 channels.
Currently all the differences are taken by comparing chip_id enum.
This patch series aims to replace chip_id with struct rcar_canfd_hw_info
to take care of the HW feature differences and driver data present
on both IPs.
The changes are trivial and tested on RZ/G2L SMARC EVK.
This patch series depend upon[1]
[1] https://lore.kernel.org/linux-renesas-soc/20221022081503.1051257-1-biju.das.jz@bp.renesas.com/T/#t
Biju Das (6):
can: rcar_canfd: rcar_canfd_probe: Add struct rcar_canfd_hw_info to
driver data
can: rcar_canfd: Add max_channels to struct rcar_canfd_hw_info
can: rcar_canfd: Add multi_global_irqs to struct rcar_canfd_hw_info
can: rcar_canfd: Add clk_postdiv to struct rcar_canfd_hw_info
can: rcar_canfd: Add multi_channel_irqs to struct rcar_canfd_hw_info
can: rcar_canfd: Add has_gerfl_eef to struct rcar_canfd_hw_info
drivers/net/can/rcar/rcar_canfd.c | 89 +++++++++++++++++++------------
1 file changed, 54 insertions(+), 35 deletions(-)
--
2.25.1
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 1/6] can: rcar_canfd: rcar_canfd_probe: Add struct rcar_canfd_hw_info to driver data
2022-10-22 10:43 [PATCH 0/6] R-Can CAN FD driver enhancements Biju Das
@ 2022-10-22 10:43 ` Biju Das
2022-10-24 14:40 ` Geert Uytterhoeven
2022-10-22 10:43 ` [PATCH 2/6] can: rcar_canfd: Add max_channels to struct rcar_canfd_hw_info Biju Das
` (4 subsequent siblings)
5 siblings, 1 reply; 18+ messages in thread
From: Biju Das @ 2022-10-22 10:43 UTC (permalink / raw)
To: Wolfgang Grandegger, Marc Kleine-Budde, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni
Cc: Biju Das, Vincent Mailhol, Stefan Mätje, Ulrich Hecht,
Lad Prabhakar, Christophe JAILLET, linux-can, netdev,
Geert Uytterhoeven, Chris Paterson, Biju Das, linux-renesas-soc
The CAN FD IP found on RZ/G2L SoC has some HW features different to that
of R-Car. For example, it has multiple resets and multiple IRQs for global
and channel interrupts. Also, it does not have ECC error flag registers
and clk post divider present on R-Car. Similarly, R-Car V3U has 8 channels
whereas other SoCs has only 2 channels.
This patch adds the struct rcar_canfd_hw_info to take care of the
HW feature differences and driver data present on both IPs. It also
replaces the driver data chip type with struct rcar_canfd_hw_info by
moving chip type to it.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
drivers/net/can/rcar/rcar_canfd.c | 43 +++++++++++++++++++++----------
1 file changed, 30 insertions(+), 13 deletions(-)
diff --git a/drivers/net/can/rcar/rcar_canfd.c b/drivers/net/can/rcar/rcar_canfd.c
index 4b12ed85deca..7bdbda1f1f12 100644
--- a/drivers/net/can/rcar/rcar_canfd.c
+++ b/drivers/net/can/rcar/rcar_canfd.c
@@ -523,6 +523,10 @@ enum rcar_canfd_fcanclk {
struct rcar_canfd_global;
+struct rcar_canfd_hw_info {
+ enum rcanfd_chip_id chip_id;
+};
+
/* Channel priv data */
struct rcar_canfd_channel {
struct can_priv can; /* Must be the first member */
@@ -548,7 +552,7 @@ struct rcar_canfd_global {
bool fdmode; /* CAN FD or Classical CAN only mode */
struct reset_control *rstc1;
struct reset_control *rstc2;
- enum rcanfd_chip_id chip_id;
+ const struct rcar_canfd_hw_info *info;
u32 max_channels;
};
@@ -591,10 +595,22 @@ static const struct can_bittiming_const rcar_canfd_bittiming_const = {
.brp_inc = 1,
};
+static const struct rcar_canfd_hw_info rcar_gen3_hw_info = {
+ .chip_id = RENESAS_RCAR_GEN3,
+};
+
+static const struct rcar_canfd_hw_info rzg2l_hw_info = {
+ .chip_id = RENESAS_RZG2L,
+};
+
+static const struct rcar_canfd_hw_info r8a779a0_hw_info = {
+ .chip_id = RENESAS_R8A779A0,
+};
+
/* Helper functions */
static inline bool is_v3u(struct rcar_canfd_global *gpriv)
{
- return gpriv->chip_id == RENESAS_R8A779A0;
+ return gpriv->info == &r8a779a0_hw_info;
}
static inline u32 reg_v3u(struct rcar_canfd_global *gpriv,
@@ -1707,6 +1723,7 @@ static const struct ethtool_ops rcar_canfd_ethtool_ops = {
static int rcar_canfd_channel_probe(struct rcar_canfd_global *gpriv, u32 ch,
u32 fcan_freq)
{
+ const struct rcar_canfd_hw_info *info = gpriv->info;
struct platform_device *pdev = gpriv->pdev;
struct rcar_canfd_channel *priv;
struct net_device *ndev;
@@ -1729,7 +1746,7 @@ static int rcar_canfd_channel_probe(struct rcar_canfd_global *gpriv, u32 ch,
priv->can.clock.freq = fcan_freq;
dev_info(&pdev->dev, "can_clk rate is %u\n", priv->can.clock.freq);
- if (gpriv->chip_id == RENESAS_RZG2L) {
+ if (info->chip_id == RENESAS_RZG2L) {
char *irq_name;
int err_irq;
int tx_irq;
@@ -1829,6 +1846,7 @@ static void rcar_canfd_channel_remove(struct rcar_canfd_global *gpriv, u32 ch)
static int rcar_canfd_probe(struct platform_device *pdev)
{
+ const struct rcar_canfd_hw_info *info;
void __iomem *addr;
u32 sts, ch, fcan_freq;
struct rcar_canfd_global *gpriv;
@@ -1837,13 +1855,12 @@ static int rcar_canfd_probe(struct platform_device *pdev)
int err, ch_irq, g_irq;
int g_err_irq, g_recc_irq;
bool fdmode = true; /* CAN FD only mode - default */
- enum rcanfd_chip_id chip_id;
int max_channels;
char name[9] = "channelX";
int i;
- chip_id = (uintptr_t)of_device_get_match_data(&pdev->dev);
- max_channels = chip_id == RENESAS_R8A779A0 ? 8 : 2;
+ info = of_device_get_match_data(&pdev->dev);
+ max_channels = info->chip_id == RENESAS_R8A779A0 ? 8 : 2;
if (of_property_read_bool(pdev->dev.of_node, "renesas,no-can-fd"))
fdmode = false; /* Classical CAN only mode */
@@ -1856,7 +1873,7 @@ static int rcar_canfd_probe(struct platform_device *pdev)
of_node_put(of_child);
}
- if (chip_id != RENESAS_RZG2L) {
+ if (info->chip_id != RENESAS_RZG2L) {
ch_irq = platform_get_irq_byname_optional(pdev, "ch_int");
if (ch_irq < 0) {
/* For backward compatibility get irq by index */
@@ -1890,7 +1907,7 @@ static int rcar_canfd_probe(struct platform_device *pdev)
gpriv->pdev = pdev;
gpriv->channels_mask = channels_mask;
gpriv->fdmode = fdmode;
- gpriv->chip_id = chip_id;
+ gpriv->info = info;
gpriv->max_channels = max_channels;
gpriv->rstc1 = devm_reset_control_get_optional_exclusive(&pdev->dev,
@@ -1928,7 +1945,7 @@ static int rcar_canfd_probe(struct platform_device *pdev)
}
fcan_freq = clk_get_rate(gpriv->can_clk);
- if (gpriv->fcan == RCANFD_CANFDCLK && gpriv->chip_id != RENESAS_RZG2L)
+ if (gpriv->fcan == RCANFD_CANFDCLK && info->chip_id != RENESAS_RZG2L)
/* CANFD clock is further divided by (1/2) within the IP */
fcan_freq /= 2;
@@ -1940,7 +1957,7 @@ static int rcar_canfd_probe(struct platform_device *pdev)
gpriv->base = addr;
/* Request IRQ that's common for both channels */
- if (gpriv->chip_id != RENESAS_RZG2L) {
+ if (info->chip_id != RENESAS_RZG2L) {
err = devm_request_irq(&pdev->dev, ch_irq,
rcar_canfd_channel_interrupt, 0,
"canfd.ch_int", gpriv);
@@ -2093,9 +2110,9 @@ static SIMPLE_DEV_PM_OPS(rcar_canfd_pm_ops, rcar_canfd_suspend,
rcar_canfd_resume);
static const __maybe_unused struct of_device_id rcar_canfd_of_table[] = {
- { .compatible = "renesas,rcar-gen3-canfd", .data = (void *)RENESAS_RCAR_GEN3 },
- { .compatible = "renesas,rzg2l-canfd", .data = (void *)RENESAS_RZG2L },
- { .compatible = "renesas,r8a779a0-canfd", .data = (void *)RENESAS_R8A779A0 },
+ { .compatible = "renesas,rcar-gen3-canfd", .data = &rcar_gen3_hw_info },
+ { .compatible = "renesas,rzg2l-canfd", .data = &rzg2l_hw_info },
+ { .compatible = "renesas,r8a779a0-canfd", .data = &r8a779a0_hw_info },
{ }
};
--
2.25.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 2/6] can: rcar_canfd: Add max_channels to struct rcar_canfd_hw_info
2022-10-22 10:43 [PATCH 0/6] R-Can CAN FD driver enhancements Biju Das
2022-10-22 10:43 ` [PATCH 1/6] can: rcar_canfd: rcar_canfd_probe: Add struct rcar_canfd_hw_info to driver data Biju Das
@ 2022-10-22 10:43 ` Biju Das
2022-10-24 14:41 ` Geert Uytterhoeven
2022-10-22 10:43 ` [PATCH 3/6] can: rcar_canfd: Add multi_global_irqs " Biju Das
` (3 subsequent siblings)
5 siblings, 1 reply; 18+ messages in thread
From: Biju Das @ 2022-10-22 10:43 UTC (permalink / raw)
To: Wolfgang Grandegger, Marc Kleine-Budde, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni
Cc: Biju Das, Vincent Mailhol, Stefan Mätje, Ulrich Hecht,
Lad Prabhakar, Christophe JAILLET, linux-can, netdev,
Geert Uytterhoeven, Chris Paterson, Biju Das, linux-renesas-soc
R-Car V3U supports a maximum of 8 channels whereas rest of the SoCs
support 2 channels.
Add max_channels variable to struct rcar_canfd_hw_info to handle this
difference.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
drivers/net/can/rcar/rcar_canfd.c | 30 +++++++++++++++---------------
1 file changed, 15 insertions(+), 15 deletions(-)
diff --git a/drivers/net/can/rcar/rcar_canfd.c b/drivers/net/can/rcar/rcar_canfd.c
index 7bdbda1f1f12..ecd7c0df2ded 100644
--- a/drivers/net/can/rcar/rcar_canfd.c
+++ b/drivers/net/can/rcar/rcar_canfd.c
@@ -525,6 +525,7 @@ struct rcar_canfd_global;
struct rcar_canfd_hw_info {
enum rcanfd_chip_id chip_id;
+ u32 max_channels;
};
/* Channel priv data */
@@ -553,7 +554,6 @@ struct rcar_canfd_global {
struct reset_control *rstc1;
struct reset_control *rstc2;
const struct rcar_canfd_hw_info *info;
- u32 max_channels;
};
/* CAN FD mode nominal rate constants */
@@ -597,14 +597,17 @@ static const struct can_bittiming_const rcar_canfd_bittiming_const = {
static const struct rcar_canfd_hw_info rcar_gen3_hw_info = {
.chip_id = RENESAS_RCAR_GEN3,
+ .max_channels = 2,
};
static const struct rcar_canfd_hw_info rzg2l_hw_info = {
.chip_id = RENESAS_RZG2L,
+ .max_channels = 2,
};
static const struct rcar_canfd_hw_info r8a779a0_hw_info = {
.chip_id = RENESAS_R8A779A0,
+ .max_channels = 8,
};
/* Helper functions */
@@ -738,7 +741,7 @@ static int rcar_canfd_reset_controller(struct rcar_canfd_global *gpriv)
rcar_canfd_set_mode(gpriv);
/* Transition all Channels to reset mode */
- for_each_set_bit(ch, &gpriv->channels_mask, gpriv->max_channels) {
+ for_each_set_bit(ch, &gpriv->channels_mask, gpriv->info->max_channels) {
rcar_canfd_clear_bit(gpriv->base,
RCANFD_CCTR(ch), RCANFD_CCTR_CSLPR);
@@ -779,7 +782,7 @@ static void rcar_canfd_configure_controller(struct rcar_canfd_global *gpriv)
rcar_canfd_set_bit(gpriv->base, RCANFD_GCFG, cfg);
/* Channel configuration settings */
- for_each_set_bit(ch, &gpriv->channels_mask, gpriv->max_channels) {
+ for_each_set_bit(ch, &gpriv->channels_mask, gpriv->info->max_channels) {
rcar_canfd_set_bit(gpriv->base, RCANFD_CCTR(ch),
RCANFD_CCTR_ERRD);
rcar_canfd_update_bit(gpriv->base, RCANFD_CCTR(ch),
@@ -1163,7 +1166,7 @@ static irqreturn_t rcar_canfd_global_err_interrupt(int irq, void *dev_id)
struct rcar_canfd_global *gpriv = dev_id;
u32 ch;
- for_each_set_bit(ch, &gpriv->channels_mask, gpriv->max_channels)
+ for_each_set_bit(ch, &gpriv->channels_mask, gpriv->info->max_channels)
rcar_canfd_handle_global_err(gpriv, ch);
return IRQ_HANDLED;
@@ -1201,7 +1204,7 @@ static irqreturn_t rcar_canfd_global_receive_fifo_interrupt(int irq, void *dev_i
struct rcar_canfd_global *gpriv = dev_id;
u32 ch;
- for_each_set_bit(ch, &gpriv->channels_mask, gpriv->max_channels)
+ for_each_set_bit(ch, &gpriv->channels_mask, gpriv->info->max_channels)
rcar_canfd_handle_global_receive(gpriv, ch);
return IRQ_HANDLED;
@@ -1215,7 +1218,7 @@ static irqreturn_t rcar_canfd_global_interrupt(int irq, void *dev_id)
/* Global error interrupts still indicate a condition specific
* to a channel. RxFIFO interrupt is a global interrupt.
*/
- for_each_set_bit(ch, &gpriv->channels_mask, gpriv->max_channels) {
+ for_each_set_bit(ch, &gpriv->channels_mask, gpriv->info->max_channels) {
rcar_canfd_handle_global_err(gpriv, ch);
rcar_canfd_handle_global_receive(gpriv, ch);
}
@@ -1311,7 +1314,7 @@ static irqreturn_t rcar_canfd_channel_interrupt(int irq, void *dev_id)
u32 ch;
/* Common FIFO is a per channel resource */
- for_each_set_bit(ch, &gpriv->channels_mask, gpriv->max_channels) {
+ for_each_set_bit(ch, &gpriv->channels_mask, gpriv->info->max_channels) {
rcar_canfd_handle_channel_err(gpriv, ch);
rcar_canfd_handle_channel_tx(gpriv, ch);
}
@@ -1855,17 +1858,15 @@ static int rcar_canfd_probe(struct platform_device *pdev)
int err, ch_irq, g_irq;
int g_err_irq, g_recc_irq;
bool fdmode = true; /* CAN FD only mode - default */
- int max_channels;
char name[9] = "channelX";
int i;
info = of_device_get_match_data(&pdev->dev);
- max_channels = info->chip_id == RENESAS_R8A779A0 ? 8 : 2;
if (of_property_read_bool(pdev->dev.of_node, "renesas,no-can-fd"))
fdmode = false; /* Classical CAN only mode */
- for (i = 0; i < max_channels; ++i) {
+ for (i = 0; i < info->max_channels; ++i) {
name[7] = '0' + i;
of_child = of_get_child_by_name(pdev->dev.of_node, name);
if (of_child && of_device_is_available(of_child))
@@ -1908,7 +1909,6 @@ static int rcar_canfd_probe(struct platform_device *pdev)
gpriv->channels_mask = channels_mask;
gpriv->fdmode = fdmode;
gpriv->info = info;
- gpriv->max_channels = max_channels;
gpriv->rstc1 = devm_reset_control_get_optional_exclusive(&pdev->dev,
"rstp_n");
@@ -2023,7 +2023,7 @@ static int rcar_canfd_probe(struct platform_device *pdev)
rcar_canfd_configure_controller(gpriv);
/* Configure per channel attributes */
- for_each_set_bit(ch, &gpriv->channels_mask, max_channels) {
+ for_each_set_bit(ch, &gpriv->channels_mask, info->max_channels) {
/* Configure Channel's Rx fifo */
rcar_canfd_configure_rx(gpriv, ch);
@@ -2049,7 +2049,7 @@ static int rcar_canfd_probe(struct platform_device *pdev)
goto fail_mode;
}
- for_each_set_bit(ch, &gpriv->channels_mask, max_channels) {
+ for_each_set_bit(ch, &gpriv->channels_mask, info->max_channels) {
err = rcar_canfd_channel_probe(gpriv, ch, fcan_freq);
if (err)
goto fail_channel;
@@ -2061,7 +2061,7 @@ static int rcar_canfd_probe(struct platform_device *pdev)
return 0;
fail_channel:
- for_each_set_bit(ch, &gpriv->channels_mask, max_channels)
+ for_each_set_bit(ch, &gpriv->channels_mask, info->max_channels)
rcar_canfd_channel_remove(gpriv, ch);
fail_mode:
rcar_canfd_disable_global_interrupts(gpriv);
@@ -2082,7 +2082,7 @@ static int rcar_canfd_remove(struct platform_device *pdev)
rcar_canfd_reset_controller(gpriv);
rcar_canfd_disable_global_interrupts(gpriv);
- for_each_set_bit(ch, &gpriv->channels_mask, gpriv->max_channels) {
+ for_each_set_bit(ch, &gpriv->channels_mask, gpriv->info->max_channels) {
rcar_canfd_disable_channel_interrupts(gpriv->ch[ch]);
rcar_canfd_channel_remove(gpriv, ch);
}
--
2.25.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 3/6] can: rcar_canfd: Add multi_global_irqs to struct rcar_canfd_hw_info
2022-10-22 10:43 [PATCH 0/6] R-Can CAN FD driver enhancements Biju Das
2022-10-22 10:43 ` [PATCH 1/6] can: rcar_canfd: rcar_canfd_probe: Add struct rcar_canfd_hw_info to driver data Biju Das
2022-10-22 10:43 ` [PATCH 2/6] can: rcar_canfd: Add max_channels to struct rcar_canfd_hw_info Biju Das
@ 2022-10-22 10:43 ` Biju Das
2022-10-24 14:45 ` Geert Uytterhoeven
2022-10-22 10:43 ` [PATCH 4/6] can: rcar_canfd: Add clk_postdiv " Biju Das
` (2 subsequent siblings)
5 siblings, 1 reply; 18+ messages in thread
From: Biju Das @ 2022-10-22 10:43 UTC (permalink / raw)
To: Wolfgang Grandegger, Marc Kleine-Budde, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni
Cc: Biju Das, Vincent Mailhol, Stefan Mätje, Ulrich Hecht,
Lad Prabhakar, Christophe JAILLET, linux-can, netdev,
Geert Uytterhoeven, Chris Paterson, Biju Das, linux-renesas-soc
RZ/G2L has separate IRQ lines for receive FIFO and global error interrupt
whereas R-Car has combined IRQ line.
Add multi_global_irqs to struct rcar_canfd_hw_info to select the driver to
choose between combined and separate irq registration for global
interrupts.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
drivers/net/can/rcar/rcar_canfd.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/net/can/rcar/rcar_canfd.c b/drivers/net/can/rcar/rcar_canfd.c
index ecd7c0df2ded..0d131694c241 100644
--- a/drivers/net/can/rcar/rcar_canfd.c
+++ b/drivers/net/can/rcar/rcar_canfd.c
@@ -526,6 +526,8 @@ struct rcar_canfd_global;
struct rcar_canfd_hw_info {
enum rcanfd_chip_id chip_id;
u32 max_channels;
+ /* hardware features */
+ unsigned multi_global_irqs:1; /* Has multiple global irqs */
};
/* Channel priv data */
@@ -603,6 +605,7 @@ static const struct rcar_canfd_hw_info rcar_gen3_hw_info = {
static const struct rcar_canfd_hw_info rzg2l_hw_info = {
.chip_id = RENESAS_RZG2L,
.max_channels = 2,
+ .multi_global_irqs = 1,
};
static const struct rcar_canfd_hw_info r8a779a0_hw_info = {
@@ -1874,7 +1877,7 @@ static int rcar_canfd_probe(struct platform_device *pdev)
of_node_put(of_child);
}
- if (info->chip_id != RENESAS_RZG2L) {
+ if (!info->multi_global_irqs) {
ch_irq = platform_get_irq_byname_optional(pdev, "ch_int");
if (ch_irq < 0) {
/* For backward compatibility get irq by index */
@@ -1957,7 +1960,7 @@ static int rcar_canfd_probe(struct platform_device *pdev)
gpriv->base = addr;
/* Request IRQ that's common for both channels */
- if (info->chip_id != RENESAS_RZG2L) {
+ if (!info->multi_global_irqs) {
err = devm_request_irq(&pdev->dev, ch_irq,
rcar_canfd_channel_interrupt, 0,
"canfd.ch_int", gpriv);
--
2.25.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 4/6] can: rcar_canfd: Add clk_postdiv to struct rcar_canfd_hw_info
2022-10-22 10:43 [PATCH 0/6] R-Can CAN FD driver enhancements Biju Das
` (2 preceding siblings ...)
2022-10-22 10:43 ` [PATCH 3/6] can: rcar_canfd: Add multi_global_irqs " Biju Das
@ 2022-10-22 10:43 ` Biju Das
2022-10-24 14:47 ` Geert Uytterhoeven
2022-10-22 10:43 ` [PATCH 5/6] can: rcar_canfd: Add multi_channel_irqs " Biju Das
2022-10-22 10:43 ` [PATCH 6/6] can: rcar_canfd: Add has_gerfl_eef " Biju Das
5 siblings, 1 reply; 18+ messages in thread
From: Biju Das @ 2022-10-22 10:43 UTC (permalink / raw)
To: Wolfgang Grandegger, Marc Kleine-Budde, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni
Cc: Biju Das, Vincent Mailhol, Stefan Mätje, Ulrich Hecht,
Lad Prabhakar, Christophe JAILLET, linux-can, netdev,
Geert Uytterhoeven, Chris Paterson, Biju Das, linux-renesas-soc
R-Car has a clock divider for CAN FD clock within the IP, whereas
it is not available on RZ/G2L.
Add clk_postdiv to struct rcar_canfd_hw_info to take care of this
difference.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
drivers/net/can/rcar/rcar_canfd.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/net/can/rcar/rcar_canfd.c b/drivers/net/can/rcar/rcar_canfd.c
index 0d131694c241..d226eb59010d 100644
--- a/drivers/net/can/rcar/rcar_canfd.c
+++ b/drivers/net/can/rcar/rcar_canfd.c
@@ -528,6 +528,7 @@ struct rcar_canfd_hw_info {
u32 max_channels;
/* hardware features */
unsigned multi_global_irqs:1; /* Has multiple global irqs */
+ unsigned clk_postdiv:1; /* Has CAN clk post divider */
};
/* Channel priv data */
@@ -600,6 +601,7 @@ static const struct can_bittiming_const rcar_canfd_bittiming_const = {
static const struct rcar_canfd_hw_info rcar_gen3_hw_info = {
.chip_id = RENESAS_RCAR_GEN3,
.max_channels = 2,
+ .clk_postdiv = 1,
};
static const struct rcar_canfd_hw_info rzg2l_hw_info = {
@@ -611,6 +613,7 @@ static const struct rcar_canfd_hw_info rzg2l_hw_info = {
static const struct rcar_canfd_hw_info r8a779a0_hw_info = {
.chip_id = RENESAS_R8A779A0,
.max_channels = 8,
+ .clk_postdiv = 1,
};
/* Helper functions */
@@ -1948,7 +1951,7 @@ static int rcar_canfd_probe(struct platform_device *pdev)
}
fcan_freq = clk_get_rate(gpriv->can_clk);
- if (gpriv->fcan == RCANFD_CANFDCLK && info->chip_id != RENESAS_RZG2L)
+ if (gpriv->fcan == RCANFD_CANFDCLK && info->clk_postdiv)
/* CANFD clock is further divided by (1/2) within the IP */
fcan_freq /= 2;
--
2.25.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 5/6] can: rcar_canfd: Add multi_channel_irqs to struct rcar_canfd_hw_info
2022-10-22 10:43 [PATCH 0/6] R-Can CAN FD driver enhancements Biju Das
` (3 preceding siblings ...)
2022-10-22 10:43 ` [PATCH 4/6] can: rcar_canfd: Add clk_postdiv " Biju Das
@ 2022-10-22 10:43 ` Biju Das
2022-10-24 14:50 ` Geert Uytterhoeven
2022-10-22 10:43 ` [PATCH 6/6] can: rcar_canfd: Add has_gerfl_eef " Biju Das
5 siblings, 1 reply; 18+ messages in thread
From: Biju Das @ 2022-10-22 10:43 UTC (permalink / raw)
To: Wolfgang Grandegger, Marc Kleine-Budde, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni
Cc: Biju Das, Vincent Mailhol, Stefan Mätje, Ulrich Hecht,
Lad Prabhakar, Christophe JAILLET, linux-can, netdev,
Geert Uytterhoeven, Chris Paterson, Biju Das, linux-renesas-soc
RZ/G2L has separate IRQ lines for tx and error interrupt for each
channel whereas R-Car has a combined IRQ line for all the channel
specific tx and error interrupts.
Add multi_channel_irqs to struct rcar_canfd_hw_info to select the
driver to choose between combined and separate irq registration for
channel interrupts. This patch also removes enum rcanfd_chip_id and
chip_id from both struct rcar_canfd_hw_info, as it is unused.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
drivers/net/can/rcar/rcar_canfd.c | 14 +++-----------
1 file changed, 3 insertions(+), 11 deletions(-)
diff --git a/drivers/net/can/rcar/rcar_canfd.c b/drivers/net/can/rcar/rcar_canfd.c
index d226eb59010d..0b6f14df2a43 100644
--- a/drivers/net/can/rcar/rcar_canfd.c
+++ b/drivers/net/can/rcar/rcar_canfd.c
@@ -41,12 +41,6 @@
#define RCANFD_DRV_NAME "rcar_canfd"
-enum rcanfd_chip_id {
- RENESAS_RCAR_GEN3 = 0,
- RENESAS_RZG2L,
- RENESAS_R8A779A0,
-};
-
/* Global register bits */
/* RSCFDnCFDGRMCFG */
@@ -524,11 +518,11 @@ enum rcar_canfd_fcanclk {
struct rcar_canfd_global;
struct rcar_canfd_hw_info {
- enum rcanfd_chip_id chip_id;
u32 max_channels;
/* hardware features */
unsigned multi_global_irqs:1; /* Has multiple global irqs */
unsigned clk_postdiv:1; /* Has CAN clk post divider */
+ unsigned multi_channel_irqs:1; /* Has multiple channel irqs */
};
/* Channel priv data */
@@ -599,19 +593,17 @@ static const struct can_bittiming_const rcar_canfd_bittiming_const = {
};
static const struct rcar_canfd_hw_info rcar_gen3_hw_info = {
- .chip_id = RENESAS_RCAR_GEN3,
.max_channels = 2,
.clk_postdiv = 1,
};
static const struct rcar_canfd_hw_info rzg2l_hw_info = {
- .chip_id = RENESAS_RZG2L,
.max_channels = 2,
.multi_global_irqs = 1,
+ .multi_channel_irqs = 1,
};
static const struct rcar_canfd_hw_info r8a779a0_hw_info = {
- .chip_id = RENESAS_R8A779A0,
.max_channels = 8,
.clk_postdiv = 1,
};
@@ -1755,7 +1747,7 @@ static int rcar_canfd_channel_probe(struct rcar_canfd_global *gpriv, u32 ch,
priv->can.clock.freq = fcan_freq;
dev_info(&pdev->dev, "can_clk rate is %u\n", priv->can.clock.freq);
- if (info->chip_id == RENESAS_RZG2L) {
+ if (info->multi_channel_irqs) {
char *irq_name;
int err_irq;
int tx_irq;
--
2.25.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 6/6] can: rcar_canfd: Add has_gerfl_eef to struct rcar_canfd_hw_info
2022-10-22 10:43 [PATCH 0/6] R-Can CAN FD driver enhancements Biju Das
` (4 preceding siblings ...)
2022-10-22 10:43 ` [PATCH 5/6] can: rcar_canfd: Add multi_channel_irqs " Biju Das
@ 2022-10-22 10:43 ` Biju Das
2022-10-24 14:59 ` Geert Uytterhoeven
5 siblings, 1 reply; 18+ messages in thread
From: Biju Das @ 2022-10-22 10:43 UTC (permalink / raw)
To: Wolfgang Grandegger, Marc Kleine-Budde, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni
Cc: Biju Das, Vincent Mailhol, Stefan Mätje, Ulrich Hecht,
Lad Prabhakar, Christophe JAILLET, linux-can, netdev,
Geert Uytterhoeven, Chris Paterson, Biju Das, linux-renesas-soc
R-Car has ECC error flags in global error interrupts whereas it is
not available on RZ/G2L.
Add has_gerfl_eef to struct rcar_canfd_hw_info so that rcar_canfd_
global_error() will process ECC errors only for R-Car.
whilst, this patch fixes the below checkpatch warnings
CHECK: Unnecessary parentheses around 'ch == 0'
CHECK: Unnecessary parentheses around 'ch == 1'
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
drivers/net/can/rcar/rcar_canfd.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/net/can/rcar/rcar_canfd.c b/drivers/net/can/rcar/rcar_canfd.c
index 0b6f14df2a43..bb825cce8acb 100644
--- a/drivers/net/can/rcar/rcar_canfd.c
+++ b/drivers/net/can/rcar/rcar_canfd.c
@@ -523,6 +523,7 @@ struct rcar_canfd_hw_info {
unsigned multi_global_irqs:1; /* Has multiple global irqs */
unsigned clk_postdiv:1; /* Has CAN clk post divider */
unsigned multi_channel_irqs:1; /* Has multiple channel irqs */
+ unsigned has_gerfl_eef:1; /* Has ECC Error Flag */
};
/* Channel priv data */
@@ -595,6 +596,7 @@ static const struct can_bittiming_const rcar_canfd_bittiming_const = {
static const struct rcar_canfd_hw_info rcar_gen3_hw_info = {
.max_channels = 2,
.clk_postdiv = 1,
+ .has_gerfl_eef = 1,
};
static const struct rcar_canfd_hw_info rzg2l_hw_info = {
@@ -606,6 +608,7 @@ static const struct rcar_canfd_hw_info rzg2l_hw_info = {
static const struct rcar_canfd_hw_info r8a779a0_hw_info = {
.max_channels = 8,
.clk_postdiv = 1,
+ .has_gerfl_eef = 1,
};
/* Helper functions */
@@ -947,17 +950,18 @@ static void rcar_canfd_global_error(struct net_device *ndev)
{
struct rcar_canfd_channel *priv = netdev_priv(ndev);
struct rcar_canfd_global *gpriv = priv->gpriv;
+ const struct rcar_canfd_hw_info *info = gpriv->info;
struct net_device_stats *stats = &ndev->stats;
u32 ch = priv->channel;
u32 gerfl, sts;
u32 ridx = ch + RCANFD_RFFIFO_IDX;
gerfl = rcar_canfd_read(priv->base, RCANFD_GERFL);
- if ((gerfl & RCANFD_GERFL_EEF0) && (ch == 0)) {
+ if (info->has_gerfl_eef && (gerfl & RCANFD_GERFL_EEF0) && ch == 0) {
netdev_dbg(ndev, "Ch0: ECC Error flag\n");
stats->tx_dropped++;
}
- if ((gerfl & RCANFD_GERFL_EEF1) && (ch == 1)) {
+ if (info->has_gerfl_eef && (gerfl & RCANFD_GERFL_EEF1) && ch == 1) {
netdev_dbg(ndev, "Ch1: ECC Error flag\n");
stats->tx_dropped++;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 1/6] can: rcar_canfd: rcar_canfd_probe: Add struct rcar_canfd_hw_info to driver data
2022-10-22 10:43 ` [PATCH 1/6] can: rcar_canfd: rcar_canfd_probe: Add struct rcar_canfd_hw_info to driver data Biju Das
@ 2022-10-24 14:40 ` Geert Uytterhoeven
2022-10-24 17:23 ` Biju Das
0 siblings, 1 reply; 18+ messages in thread
From: Geert Uytterhoeven @ 2022-10-24 14:40 UTC (permalink / raw)
To: biju.das.jz
Cc: Wolfgang Grandegger, Marc Kleine-Budde, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Vincent Mailhol,
Stefan Mätje, Ulrich Hecht, Lad Prabhakar,
Christophe JAILLET, linux-can, netdev, Chris Paterson, Biju Das,
linux-renesas-soc
Hi Biju,
On Sat, Oct 22, 2022 at 1:02 PM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> The CAN FD IP found on RZ/G2L SoC has some HW features different to that
> of R-Car. For example, it has multiple resets and multiple IRQs for global
> and channel interrupts. Also, it does not have ECC error flag registers
> and clk post divider present on R-Car. Similarly, R-Car V3U has 8 channels
> whereas other SoCs has only 2 channels.
>
> This patch adds the struct rcar_canfd_hw_info to take care of the
> HW feature differences and driver data present on both IPs. It also
> replaces the driver data chip type with struct rcar_canfd_hw_info by
> moving chip type to it.
>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Thanks for your patch!
> --- a/drivers/net/can/rcar/rcar_canfd.c
> +++ b/drivers/net/can/rcar/rcar_canfd.c
> @@ -591,10 +595,22 @@ static const struct can_bittiming_const rcar_canfd_bittiming_const = {
> .brp_inc = 1,
> };
>
> +static const struct rcar_canfd_hw_info rcar_gen3_hw_info = {
> + .chip_id = RENESAS_RCAR_GEN3,
> +};
> +
> +static const struct rcar_canfd_hw_info rzg2l_hw_info = {
> + .chip_id = RENESAS_RZG2L,
> +};
> +
> +static const struct rcar_canfd_hw_info r8a779a0_hw_info = {
> + .chip_id = RENESAS_R8A779A0,
> +};
> +
> /* Helper functions */
> static inline bool is_v3u(struct rcar_canfd_global *gpriv)
> {
> - return gpriv->chip_id == RENESAS_R8A779A0;
> + return gpriv->info == &r8a779a0_hw_info;
"return gpriv->info->chip_id == RENESAS_R8A779A0;" would match all
the other changes you make. But I see why you did it this way...
((most) users of is_v3u() are not converted to feature flags (yet) ;-)
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/6] can: rcar_canfd: Add max_channels to struct rcar_canfd_hw_info
2022-10-22 10:43 ` [PATCH 2/6] can: rcar_canfd: Add max_channels to struct rcar_canfd_hw_info Biju Das
@ 2022-10-24 14:41 ` Geert Uytterhoeven
2022-10-24 17:17 ` Biju Das
0 siblings, 1 reply; 18+ messages in thread
From: Geert Uytterhoeven @ 2022-10-24 14:41 UTC (permalink / raw)
To: biju.das.jz
Cc: Wolfgang Grandegger, Marc Kleine-Budde, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Vincent Mailhol,
Stefan Mätje, Ulrich Hecht, Lad Prabhakar,
Christophe JAILLET, linux-can, netdev, Geert Uytterhoeven,
Chris Paterson, Biju Das, linux-renesas-soc
On Sat, Oct 22, 2022 at 1:13 PM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> R-Car V3U supports a maximum of 8 channels whereas rest of the SoCs
> support 2 channels.
>
> Add max_channels variable to struct rcar_canfd_hw_info to handle this
> difference.
>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> --- a/drivers/net/can/rcar/rcar_canfd.c
> +++ b/drivers/net/can/rcar/rcar_canfd.c
> @@ -525,6 +525,7 @@ struct rcar_canfd_global;
>
> struct rcar_canfd_hw_info {
> enum rcanfd_chip_id chip_id;
> + u32 max_channels;
Although I wouldn't mind "unsigned int" instead...
> };
>
> /* Channel priv data */
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 3/6] can: rcar_canfd: Add multi_global_irqs to struct rcar_canfd_hw_info
2022-10-22 10:43 ` [PATCH 3/6] can: rcar_canfd: Add multi_global_irqs " Biju Das
@ 2022-10-24 14:45 ` Geert Uytterhoeven
2022-10-24 17:19 ` Biju Das
0 siblings, 1 reply; 18+ messages in thread
From: Geert Uytterhoeven @ 2022-10-24 14:45 UTC (permalink / raw)
To: biju.das.jz
Cc: Wolfgang Grandegger, Marc Kleine-Budde, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Vincent Mailhol,
Stefan Mätje, Ulrich Hecht, Lad Prabhakar,
Christophe JAILLET, linux-can, netdev, Chris Paterson, Biju Das,
linux-renesas-soc
Hi Biju,
On Sat, Oct 22, 2022 at 1:03 PM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> RZ/G2L has separate IRQ lines for receive FIFO and global error interrupt
> whereas R-Car has combined IRQ line.
>
> Add multi_global_irqs to struct rcar_canfd_hw_info to select the driver to
> choose between combined and separate irq registration for global
> interrupts.
>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Thanks for your patch!
> --- a/drivers/net/can/rcar/rcar_canfd.c
> +++ b/drivers/net/can/rcar/rcar_canfd.c
> @@ -526,6 +526,8 @@ struct rcar_canfd_global;
> struct rcar_canfd_hw_info {
> enum rcanfd_chip_id chip_id;
> u32 max_channels;
> + /* hardware features */
> + unsigned multi_global_irqs:1; /* Has multiple global irqs */
I'm not sure this is the best name for this flag (especially considering
the other flag being added in [5/6])...
> };
>
> /* Channel priv data */
> @@ -603,6 +605,7 @@ static const struct rcar_canfd_hw_info rcar_gen3_hw_info = {
> static const struct rcar_canfd_hw_info rzg2l_hw_info = {
> .chip_id = RENESAS_RZG2L,
> .max_channels = 2,
> + .multi_global_irqs = 1,
> };
>
> static const struct rcar_canfd_hw_info r8a779a0_hw_info = {
> @@ -1874,7 +1877,7 @@ static int rcar_canfd_probe(struct platform_device *pdev)
> of_node_put(of_child);
> }
>
> - if (info->chip_id != RENESAS_RZG2L) {
> + if (!info->multi_global_irqs) {
> ch_irq = platform_get_irq_byname_optional(pdev, "ch_int");
> if (ch_irq < 0) {
> /* For backward compatibility get irq by index */
> @@ -1957,7 +1960,7 @@ static int rcar_canfd_probe(struct platform_device *pdev)
> gpriv->base = addr;
>
> /* Request IRQ that's common for both channels */
> - if (info->chip_id != RENESAS_RZG2L) {
> + if (!info->multi_global_irqs) {
All checks for this flag are negative checks. What about inverting
the meaning of the flag, so these can become positive checks?
> err = devm_request_irq(&pdev->dev, ch_irq,
> rcar_canfd_channel_interrupt, 0,
> "canfd.ch_int", gpriv);
As the patch itself is correct:
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 4/6] can: rcar_canfd: Add clk_postdiv to struct rcar_canfd_hw_info
2022-10-22 10:43 ` [PATCH 4/6] can: rcar_canfd: Add clk_postdiv " Biju Das
@ 2022-10-24 14:47 ` Geert Uytterhoeven
2022-10-24 17:25 ` Biju Das
0 siblings, 1 reply; 18+ messages in thread
From: Geert Uytterhoeven @ 2022-10-24 14:47 UTC (permalink / raw)
To: biju.das.jz
Cc: Wolfgang Grandegger, Marc Kleine-Budde, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Vincent Mailhol,
Stefan Mätje, Ulrich Hecht, Lad Prabhakar,
Christophe JAILLET, linux-can, netdev, Geert Uytterhoeven,
Chris Paterson, Biju Das, linux-renesas-soc
Hi Biju,
On Sat, Oct 22, 2022 at 1:03 PM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> R-Car has a clock divider for CAN FD clock within the IP, whereas
> it is not available on RZ/G2L.
>
> Add clk_postdiv to struct rcar_canfd_hw_info to take care of this
> difference.
>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Thanks for your patch!
> --- a/drivers/net/can/rcar/rcar_canfd.c
> +++ b/drivers/net/can/rcar/rcar_canfd.c
> @@ -528,6 +528,7 @@ struct rcar_canfd_hw_info {
> u32 max_channels;
> /* hardware features */
> unsigned multi_global_irqs:1; /* Has multiple global irqs */
> + unsigned clk_postdiv:1; /* Has CAN clk post divider */
As this is not the actual post divider, I think this should be called
has_clk_postdiv. But see below...
> };
>
> /* Channel priv data */
> @@ -1948,7 +1951,7 @@ static int rcar_canfd_probe(struct platform_device *pdev)
> }
> fcan_freq = clk_get_rate(gpriv->can_clk);
>
> - if (gpriv->fcan == RCANFD_CANFDCLK && info->chip_id != RENESAS_RZG2L)
> + if (gpriv->fcan == RCANFD_CANFDCLK && info->clk_postdiv)
> /* CANFD clock is further divided by (1/2) within the IP */
> fcan_freq /= 2;
If info->clk_postdiv would be the actual post divider, you could simplify to:
if (gpriv->fcan == RCANFD_CANFDCLK)
fcan_freq /= info->postdiv;
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 5/6] can: rcar_canfd: Add multi_channel_irqs to struct rcar_canfd_hw_info
2022-10-22 10:43 ` [PATCH 5/6] can: rcar_canfd: Add multi_channel_irqs " Biju Das
@ 2022-10-24 14:50 ` Geert Uytterhoeven
0 siblings, 0 replies; 18+ messages in thread
From: Geert Uytterhoeven @ 2022-10-24 14:50 UTC (permalink / raw)
To: biju.das.jz
Cc: Wolfgang Grandegger, Marc Kleine-Budde, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Vincent Mailhol,
Stefan Mätje, Ulrich Hecht, Lad Prabhakar,
Christophe JAILLET, linux-can, netdev, Chris Paterson, Biju Das,
linux-renesas-soc
On Sat, Oct 22, 2022 at 1:03 PM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> RZ/G2L has separate IRQ lines for tx and error interrupt for each
> channel whereas R-Car has a combined IRQ line for all the channel
> specific tx and error interrupts.
>
> Add multi_channel_irqs to struct rcar_canfd_hw_info to select the
> driver to choose between combined and separate irq registration for
> channel interrupts. This patch also removes enum rcanfd_chip_id and
> chip_id from both struct rcar_canfd_hw_info, as it is unused.
>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 6/6] can: rcar_canfd: Add has_gerfl_eef to struct rcar_canfd_hw_info
2022-10-22 10:43 ` [PATCH 6/6] can: rcar_canfd: Add has_gerfl_eef " Biju Das
@ 2022-10-24 14:59 ` Geert Uytterhoeven
2022-10-24 17:46 ` Biju Das
0 siblings, 1 reply; 18+ messages in thread
From: Geert Uytterhoeven @ 2022-10-24 14:59 UTC (permalink / raw)
To: biju.das.jz
Cc: Wolfgang Grandegger, Marc Kleine-Budde, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Vincent Mailhol,
Stefan Mätje, Ulrich Hecht, Lad Prabhakar,
Christophe JAILLET, linux-can, netdev, Chris Paterson, Biju Das,
linux-renesas-soc
Hi Biju,
On Sat, Oct 22, 2022 at 1:03 PM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> R-Car has ECC error flags in global error interrupts whereas it is
> not available on RZ/G2L.
>
> Add has_gerfl_eef to struct rcar_canfd_hw_info so that rcar_canfd_
> global_error() will process ECC errors only for R-Car.
>
> whilst, this patch fixes the below checkpatch warnings
> CHECK: Unnecessary parentheses around 'ch == 0'
> CHECK: Unnecessary parentheses around 'ch == 1'
>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Thanks for your patch!
> --- a/drivers/net/can/rcar/rcar_canfd.c
> +++ b/drivers/net/can/rcar/rcar_canfd.c
> @@ -523,6 +523,7 @@ struct rcar_canfd_hw_info {
> unsigned multi_global_irqs:1; /* Has multiple global irqs */
> unsigned clk_postdiv:1; /* Has CAN clk post divider */
> unsigned multi_channel_irqs:1; /* Has multiple channel irqs */
> + unsigned has_gerfl_eef:1; /* Has ECC Error Flag */
Do you really need this flag? According to the RZ/G2L docs, the
corresponding register bits are always read as zero.
> };
>
> /* Channel priv data */
> @@ -947,17 +950,18 @@ static void rcar_canfd_global_error(struct net_device *ndev)
> {
> struct rcar_canfd_channel *priv = netdev_priv(ndev);
> struct rcar_canfd_global *gpriv = priv->gpriv;
> + const struct rcar_canfd_hw_info *info = gpriv->info;
> struct net_device_stats *stats = &ndev->stats;
> u32 ch = priv->channel;
> u32 gerfl, sts;
> u32 ridx = ch + RCANFD_RFFIFO_IDX;
>
> gerfl = rcar_canfd_read(priv->base, RCANFD_GERFL);
> - if ((gerfl & RCANFD_GERFL_EEF0) && (ch == 0)) {
> + if (info->has_gerfl_eef && (gerfl & RCANFD_GERFL_EEF0) && ch == 0) {
> netdev_dbg(ndev, "Ch0: ECC Error flag\n");
> stats->tx_dropped++;
> }
> - if ((gerfl & RCANFD_GERFL_EEF1) && (ch == 1)) {
> + if (info->has_gerfl_eef && (gerfl & RCANFD_GERFL_EEF1) && ch == 1) {
> netdev_dbg(ndev, "Ch1: ECC Error flag\n");
> stats->tx_dropped++;
> }
Just wrap both checks inside a single "if (gpriv->info->has_gerfl) { ... }"?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 18+ messages in thread
* RE: [PATCH 2/6] can: rcar_canfd: Add max_channels to struct rcar_canfd_hw_info
2022-10-24 14:41 ` Geert Uytterhoeven
@ 2022-10-24 17:17 ` Biju Das
0 siblings, 0 replies; 18+ messages in thread
From: Biju Das @ 2022-10-24 17:17 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Wolfgang Grandegger, Marc Kleine-Budde, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Vincent Mailhol,
Stefan Mätje, Ulrich Hecht, Prabhakar Mahadev Lad,
Christophe JAILLET, linux-can, netdev, Geert Uytterhoeven,
Chris Paterson, Biju Das, linux-renesas-soc
Hi Geert,
Thanks for the feedback.
> <biju.das@bp.renesas.com>; linux-renesas-soc@vger.kernel.org
> Subject: Re: [PATCH 2/6] can: rcar_canfd: Add max_channels to struct
> rcar_canfd_hw_info
>
> On Sat, Oct 22, 2022 at 1:13 PM Biju Das <biju.das.jz@bp.renesas.com>
> wrote:
> > R-Car V3U supports a maximum of 8 channels whereas rest of the SoCs
> > support 2 channels.
> >
> > Add max_channels variable to struct rcar_canfd_hw_info to handle
> this
> > difference.
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
>
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
>
> > --- a/drivers/net/can/rcar/rcar_canfd.c
> > +++ b/drivers/net/can/rcar/rcar_canfd.c
> > @@ -525,6 +525,7 @@ struct rcar_canfd_global;
> >
> > struct rcar_canfd_hw_info {
> > enum rcanfd_chip_id chip_id;
> > + u32 max_channels;
>
> Although I wouldn't mind "unsigned int" instead...
OK. Agreed will change it to unsigned int.
Cheers,
Biju
^ permalink raw reply [flat|nested] 18+ messages in thread
* RE: [PATCH 3/6] can: rcar_canfd: Add multi_global_irqs to struct rcar_canfd_hw_info
2022-10-24 14:45 ` Geert Uytterhoeven
@ 2022-10-24 17:19 ` Biju Das
0 siblings, 0 replies; 18+ messages in thread
From: Biju Das @ 2022-10-24 17:19 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Wolfgang Grandegger, Marc Kleine-Budde, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Vincent Mailhol,
Stefan Mätje, Ulrich Hecht, Prabhakar Mahadev Lad,
Christophe JAILLET, linux-can, netdev, Chris Paterson, Biju Das,
linux-renesas-soc
Hi Geert,
Thanks for the feedback.
> Subject: Re: [PATCH 3/6] can: rcar_canfd: Add multi_global_irqs to
> struct rcar_canfd_hw_info
>
> Hi Biju,
>
> On Sat, Oct 22, 2022 at 1:03 PM Biju Das <biju.das.jz@bp.renesas.com>
> wrote:
> > RZ/G2L has separate IRQ lines for receive FIFO and global error
> > interrupt whereas R-Car has combined IRQ line.
> >
> > Add multi_global_irqs to struct rcar_canfd_hw_info to select the
> > driver to choose between combined and separate irq registration for
> > global interrupts.
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
>
> Thanks for your patch!
>
> > --- a/drivers/net/can/rcar/rcar_canfd.c
> > +++ b/drivers/net/can/rcar/rcar_canfd.c
> > @@ -526,6 +526,8 @@ struct rcar_canfd_global; struct
> > rcar_canfd_hw_info {
> > enum rcanfd_chip_id chip_id;
> > u32 max_channels;
> > + /* hardware features */
> > + unsigned multi_global_irqs:1; /* Has multiple global irqs
> */
>
> I'm not sure this is the best name for this flag (especially
> considering the other flag being added in [5/6])...
OK.
>
> > };
> >
> > /* Channel priv data */
> > @@ -603,6 +605,7 @@ static const struct rcar_canfd_hw_info
> > rcar_gen3_hw_info = { static const struct rcar_canfd_hw_info
> rzg2l_hw_info = {
> > .chip_id = RENESAS_RZG2L,
> > .max_channels = 2,
> > + .multi_global_irqs = 1,
> > };
> >
> > static const struct rcar_canfd_hw_info r8a779a0_hw_info = { @@
> > -1874,7 +1877,7 @@ static int rcar_canfd_probe(struct
> platform_device *pdev)
> > of_node_put(of_child);
> > }
> >
> > - if (info->chip_id != RENESAS_RZG2L) {
> > + if (!info->multi_global_irqs) {
> > ch_irq = platform_get_irq_byname_optional(pdev,
> "ch_int");
> > if (ch_irq < 0) {
> > /* For backward compatibility get irq by
> index
> > */ @@ -1957,7 +1960,7 @@ static int rcar_canfd_probe(struct
> platform_device *pdev)
> > gpriv->base = addr;
> >
> > /* Request IRQ that's common for both channels */
> > - if (info->chip_id != RENESAS_RZG2L) {
> > + if (!info->multi_global_irqs) {
>
> All checks for this flag are negative checks. What about inverting
> the meaning of the flag, so these can become positive checks?
OK, will change it to shared_global_irqs as it is shared between
Global receive and error interrupts.
Cheers,
Biju
>
> > err = devm_request_irq(&pdev->dev, ch_irq,
> > rcar_canfd_channel_interrupt,
> 0,
> > "canfd.ch_int", gpriv);
>
> As the patch itself is correct:
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 --
> geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a
> hacker. But when I'm talking to journalists I just say "programmer" or
> something like that.
> -- Linus Torvalds
^ permalink raw reply [flat|nested] 18+ messages in thread
* RE: [PATCH 1/6] can: rcar_canfd: rcar_canfd_probe: Add struct rcar_canfd_hw_info to driver data
2022-10-24 14:40 ` Geert Uytterhoeven
@ 2022-10-24 17:23 ` Biju Das
0 siblings, 0 replies; 18+ messages in thread
From: Biju Das @ 2022-10-24 17:23 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Wolfgang Grandegger, Marc Kleine-Budde, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Vincent Mailhol,
Stefan Mätje, Ulrich Hecht, Prabhakar Mahadev Lad,
Christophe JAILLET, linux-can, netdev, Chris Paterson, Biju Das,
linux-renesas-soc
Hi Geert,
Thanks for the feedback.
> Subject: Re: [PATCH 1/6] can: rcar_canfd: rcar_canfd_probe: Add struct
> rcar_canfd_hw_info to driver data
>
> Hi Biju,
>
> On Sat, Oct 22, 2022 at 1:02 PM Biju Das <biju.das.jz@bp.renesas.com>
> wrote:
> > The CAN FD IP found on RZ/G2L SoC has some HW features different to
> > that of R-Car. For example, it has multiple resets and multiple IRQs
> > for global and channel interrupts. Also, it does not have ECC error
> > flag registers and clk post divider present on R-Car. Similarly, R-
> Car
> > V3U has 8 channels whereas other SoCs has only 2 channels.
> >
> > This patch adds the struct rcar_canfd_hw_info to take care of the HW
> > feature differences and driver data present on both IPs. It also
> > replaces the driver data chip type with struct rcar_canfd_hw_info by
> > moving chip type to it.
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
>
> Thanks for your patch!
>
> > --- a/drivers/net/can/rcar/rcar_canfd.c
> > +++ b/drivers/net/can/rcar/rcar_canfd.c
>
> > @@ -591,10 +595,22 @@ static const struct can_bittiming_const
> rcar_canfd_bittiming_const = {
> > .brp_inc = 1,
> > };
> >
> > +static const struct rcar_canfd_hw_info rcar_gen3_hw_info = {
> > + .chip_id = RENESAS_RCAR_GEN3,
> > +};
> > +
> > +static const struct rcar_canfd_hw_info rzg2l_hw_info = {
> > + .chip_id = RENESAS_RZG2L,
> > +};
> > +
> > +static const struct rcar_canfd_hw_info r8a779a0_hw_info = {
> > + .chip_id = RENESAS_R8A779A0,
> > +};
> > +
> > /* Helper functions */
> > static inline bool is_v3u(struct rcar_canfd_global *gpriv) {
> > - return gpriv->chip_id == RENESAS_R8A779A0;
> > + return gpriv->info == &r8a779a0_hw_info;
>
> "return gpriv->info->chip_id == RENESAS_R8A779A0;" would match all the
> other changes you make. But I see why you did it this way...
> ((most) users of is_v3u() are not converted to feature flags (yet) ;-)
Yep, that is correct. Chip_id() is being removed in subsequent patches
Otherwise I need to create separate patch for this change alone.
Cheers,
Biju
>
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 --
> geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a
> hacker. But when I'm talking to journalists I just say "programmer" or
> something like that.
> -- Linus Torvalds
^ permalink raw reply [flat|nested] 18+ messages in thread
* RE: [PATCH 4/6] can: rcar_canfd: Add clk_postdiv to struct rcar_canfd_hw_info
2022-10-24 14:47 ` Geert Uytterhoeven
@ 2022-10-24 17:25 ` Biju Das
0 siblings, 0 replies; 18+ messages in thread
From: Biju Das @ 2022-10-24 17:25 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Wolfgang Grandegger, Marc Kleine-Budde, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Vincent Mailhol,
Stefan Mätje, Ulrich Hecht, Prabhakar Mahadev Lad,
Christophe JAILLET, linux-can, netdev, Geert Uytterhoeven,
Chris Paterson, Biju Das, linux-renesas-soc
Hi Geert,
Thanks for the feedback.
> Subject: Re: [PATCH 4/6] can: rcar_canfd: Add clk_postdiv to struct
> rcar_canfd_hw_info
>
> Hi Biju,
>
> On Sat, Oct 22, 2022 at 1:03 PM Biju Das <biju.das.jz@bp.renesas.com>
> wrote:
> > R-Car has a clock divider for CAN FD clock within the IP, whereas it
> > is not available on RZ/G2L.
> >
> > Add clk_postdiv to struct rcar_canfd_hw_info to take care of this
> > difference.
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
>
> Thanks for your patch!
>
> > --- a/drivers/net/can/rcar/rcar_canfd.c
> > +++ b/drivers/net/can/rcar/rcar_canfd.c
> > @@ -528,6 +528,7 @@ struct rcar_canfd_hw_info {
> > u32 max_channels;
> > /* hardware features */
> > unsigned multi_global_irqs:1; /* Has multiple global irqs
> */
> > + unsigned clk_postdiv:1; /* Has CAN clk post divider
> */
>
> As this is not the actual post divider, I think this should be called
> has_clk_postdiv. But see below...
OK will use driver data " postdiv" instead as mentioned below.
>
> > };
> >
> > /* Channel priv data */
>
> > @@ -1948,7 +1951,7 @@ static int rcar_canfd_probe(struct
> platform_device *pdev)
> > }
> > fcan_freq = clk_get_rate(gpriv->can_clk);
> >
> > - if (gpriv->fcan == RCANFD_CANFDCLK && info->chip_id !=
> RENESAS_RZG2L)
> > + if (gpriv->fcan == RCANFD_CANFDCLK && info->clk_postdiv)
> > /* CANFD clock is further divided by (1/2) within
> the IP */
> > fcan_freq /= 2;
>
> If info->clk_postdiv would be the actual post divider, you could
> simplify to:
>
> if (gpriv->fcan == RCANFD_CANFDCLK)
> fcan_freq /= info->postdiv;
Agreed.
Thanks and regards,
Biju
^ permalink raw reply [flat|nested] 18+ messages in thread
* RE: [PATCH 6/6] can: rcar_canfd: Add has_gerfl_eef to struct rcar_canfd_hw_info
2022-10-24 14:59 ` Geert Uytterhoeven
@ 2022-10-24 17:46 ` Biju Das
0 siblings, 0 replies; 18+ messages in thread
From: Biju Das @ 2022-10-24 17:46 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Wolfgang Grandegger, Marc Kleine-Budde, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Vincent Mailhol,
Stefan Mätje, Ulrich Hecht, Prabhakar Mahadev Lad,
Christophe JAILLET, linux-can, netdev, Chris Paterson, Biju Das,
linux-renesas-soc
Hi Geert,
Thanks for the feedback.
> Subject: Re: [PATCH 6/6] can: rcar_canfd: Add has_gerfl_eef to struct
> rcar_canfd_hw_info
>
> Hi Biju,
>
> On Sat, Oct 22, 2022 at 1:03 PM Biju Das <biju.das.jz@bp.renesas.com>
> wrote:
> > R-Car has ECC error flags in global error interrupts whereas it is
> not
> > available on RZ/G2L.
> >
> > Add has_gerfl_eef to struct rcar_canfd_hw_info so that rcar_canfd_
> > global_error() will process ECC errors only for R-Car.
> >
> > whilst, this patch fixes the below checkpatch warnings
> > CHECK: Unnecessary parentheses around 'ch == 0'
> > CHECK: Unnecessary parentheses around 'ch == 1'
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
>
> Thanks for your patch!
>
> > --- a/drivers/net/can/rcar/rcar_canfd.c
> > +++ b/drivers/net/can/rcar/rcar_canfd.c
> > @@ -523,6 +523,7 @@ struct rcar_canfd_hw_info {
> > unsigned multi_global_irqs:1; /* Has multiple global irqs
> */
> > unsigned clk_postdiv:1; /* Has CAN clk post divider
> */
> > unsigned multi_channel_irqs:1; /* Has multiple channel irqs
> > */
> > + unsigned has_gerfl_eef:1; /* Has ECC Error Flag */
>
> Do you really need this flag? According to the RZ/G2L docs, the
> corresponding register bits are always read as zero.
These are reserved bits with 0 value, But it is not documented as ECC Error flag for RZ/G2L.
So just want to be clear it is a hw feature that not supported for RZ/G2L.
>
> > };
> >
> > /* Channel priv data */
>
> > @@ -947,17 +950,18 @@ static void rcar_canfd_global_error(struct
> > net_device *ndev) {
> > struct rcar_canfd_channel *priv = netdev_priv(ndev);
> > struct rcar_canfd_global *gpriv = priv->gpriv;
> > + const struct rcar_canfd_hw_info *info = gpriv->info;
> > struct net_device_stats *stats = &ndev->stats;
> > u32 ch = priv->channel;
> > u32 gerfl, sts;
> > u32 ridx = ch + RCANFD_RFFIFO_IDX;
> >
> > gerfl = rcar_canfd_read(priv->base, RCANFD_GERFL);
> > - if ((gerfl & RCANFD_GERFL_EEF0) && (ch == 0)) {
> > + if (info->has_gerfl_eef && (gerfl & RCANFD_GERFL_EEF0) && ch
> > + == 0) {
> > netdev_dbg(ndev, "Ch0: ECC Error flag\n");
> > stats->tx_dropped++;
> > }
> > - if ((gerfl & RCANFD_GERFL_EEF1) && (ch == 1)) {
> > + if (info->has_gerfl_eef && (gerfl & RCANFD_GERFL_EEF1) && ch
> > + == 1) {
> > netdev_dbg(ndev, "Ch1: ECC Error flag\n");
> > stats->tx_dropped++;
> > }
>
> Just wrap both checks inside a single "if (gpriv->info->has_gerfl) {
> ... }"?
>
OK, cheers,
Biju
^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2022-10-24 19:21 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-22 10:43 [PATCH 0/6] R-Can CAN FD driver enhancements Biju Das
2022-10-22 10:43 ` [PATCH 1/6] can: rcar_canfd: rcar_canfd_probe: Add struct rcar_canfd_hw_info to driver data Biju Das
2022-10-24 14:40 ` Geert Uytterhoeven
2022-10-24 17:23 ` Biju Das
2022-10-22 10:43 ` [PATCH 2/6] can: rcar_canfd: Add max_channels to struct rcar_canfd_hw_info Biju Das
2022-10-24 14:41 ` Geert Uytterhoeven
2022-10-24 17:17 ` Biju Das
2022-10-22 10:43 ` [PATCH 3/6] can: rcar_canfd: Add multi_global_irqs " Biju Das
2022-10-24 14:45 ` Geert Uytterhoeven
2022-10-24 17:19 ` Biju Das
2022-10-22 10:43 ` [PATCH 4/6] can: rcar_canfd: Add clk_postdiv " Biju Das
2022-10-24 14:47 ` Geert Uytterhoeven
2022-10-24 17:25 ` Biju Das
2022-10-22 10:43 ` [PATCH 5/6] can: rcar_canfd: Add multi_channel_irqs " Biju Das
2022-10-24 14:50 ` Geert Uytterhoeven
2022-10-22 10:43 ` [PATCH 6/6] can: rcar_canfd: Add has_gerfl_eef " Biju Das
2022-10-24 14:59 ` Geert Uytterhoeven
2022-10-24 17:46 ` Biju Das
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).