| Message ID | 20210721194951.30983-3-prabhakar.mahadev-lad.rj@bp.renesas.com |
|---|---|
| State | New |
| Headers | show |
| Series | Renesas RZ/G2L CANFD support | expand |
Hi Lad,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on renesas-devel/next]
[also build test WARNING on v5.14-rc2 next-20210723]
[cannot apply to mkl-can-next/testing robh/for-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Lad-Prabhakar/Renesas-RZ-G2L-CANFD-support/20210722-035332
base: https://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-devel.git next
config: arm64-randconfig-r031-20210723 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 9625ca5b602616b2f5584e8a49ba93c52c141e40)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install arm64 cross compiling tool for clang build
# apt-get install binutils-aarch64-linux-gnu
# https://github.com/0day-ci/linux/commit/082d605e73c5922419a736aa9ecd3a76c0241bf7
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Lad-Prabhakar/Renesas-RZ-G2L-CANFD-support/20210722-035332
git checkout 082d605e73c5922419a736aa9ecd3a76c0241bf7
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=arm64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> drivers/net/can/rcar/rcar_canfd.c:1699:12: warning: cast to smaller integer type 'enum rcanfd_chip_id' from 'const void *' [-Wvoid-pointer-to-enum-cast]
chip_id = (enum rcanfd_chip_id)of_device_get_match_data(&pdev->dev);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 warning generated.
vim +1699 drivers/net/can/rcar/rcar_canfd.c
1686
1687 static int rcar_canfd_probe(struct platform_device *pdev)
1688 {
1689 void __iomem *addr;
1690 u32 sts, ch, fcan_freq;
1691 struct rcar_canfd_global *gpriv;
1692 struct device_node *of_child;
1693 unsigned long channels_mask = 0;
1694 int err, ch_irq, g_irq;
1695 int g_err_irq, g_recc_irq;
1696 bool fdmode = true; /* CAN FD only mode - default */
1697 enum rcanfd_chip_id chip_id;
1698
> 1699 chip_id = (enum rcanfd_chip_id)of_device_get_match_data(&pdev->dev);
1700
1701 if (of_property_read_bool(pdev->dev.of_node, "renesas,no-can-fd"))
1702 fdmode = false; /* Classical CAN only mode */
1703
1704 of_child = of_get_child_by_name(pdev->dev.of_node, "channel0");
1705 if (of_child && of_device_is_available(of_child))
1706 channels_mask |= BIT(0); /* Channel 0 */
1707
1708 of_child = of_get_child_by_name(pdev->dev.of_node, "channel1");
1709 if (of_child && of_device_is_available(of_child))
1710 channels_mask |= BIT(1); /* Channel 1 */
1711
1712 if (chip_id == RENESAS_RCAR_GEN3) {
1713 ch_irq = platform_get_irq_byname_optional(pdev, "ch_int");
1714 if (ch_irq < 0) {
1715 /* For backward compatibility get irq by index */
1716 ch_irq = platform_get_irq(pdev, 0);
1717 if (ch_irq < 0)
1718 return ch_irq;
1719 }
1720
1721 g_irq = platform_get_irq_byname_optional(pdev, "g_int");
1722 if (g_irq < 0) {
1723 /* For backward compatibility get irq by index */
1724 g_irq = platform_get_irq(pdev, 1);
1725 if (g_irq < 0)
1726 return g_irq;
1727 }
1728 } else {
1729 g_err_irq = platform_get_irq_byname(pdev, "g_err");
1730 if (g_err_irq < 0)
1731 return g_err_irq;
1732
1733 g_recc_irq = platform_get_irq_byname(pdev, "g_recc");
1734 if (g_recc_irq < 0)
1735 return g_recc_irq;
1736 }
1737
1738 /* Global controller context */
1739 gpriv = devm_kzalloc(&pdev->dev, sizeof(*gpriv), GFP_KERNEL);
1740 if (!gpriv) {
1741 err = -ENOMEM;
1742 goto fail_dev;
1743 }
1744 gpriv->pdev = pdev;
1745 gpriv->channels_mask = channels_mask;
1746 gpriv->fdmode = fdmode;
1747 gpriv->chip_id = chip_id;
1748
1749 if (gpriv->chip_id == RENESAS_RZG2L) {
1750 gpriv->rstc1 = devm_reset_control_get_exclusive(&pdev->dev, "rstp_n");
1751 if (IS_ERR(gpriv->rstc1))
1752 return dev_err_probe(&pdev->dev, PTR_ERR(gpriv->rstc1),
1753 "failed to get rstp_n\n");
1754
1755 gpriv->rstc2 = devm_reset_control_get_exclusive(&pdev->dev, "rstc_n");
1756 if (IS_ERR(gpriv->rstc2))
1757 return dev_err_probe(&pdev->dev, PTR_ERR(gpriv->rstc2),
1758 "failed to get rstc_n\n");
1759 }
1760
1761 /* Peripheral clock */
1762 gpriv->clkp = devm_clk_get(&pdev->dev, "fck");
1763 if (IS_ERR(gpriv->clkp)) {
1764 err = PTR_ERR(gpriv->clkp);
1765 dev_err(&pdev->dev, "cannot get peripheral clock, error %d\n",
1766 err);
1767 goto fail_dev;
1768 }
1769
1770 /* fCAN clock: Pick External clock. If not available fallback to
1771 * CANFD clock
1772 */
1773 gpriv->can_clk = devm_clk_get(&pdev->dev, "can_clk");
1774 if (IS_ERR(gpriv->can_clk) || (clk_get_rate(gpriv->can_clk) == 0)) {
1775 gpriv->can_clk = devm_clk_get(&pdev->dev, "canfd");
1776 if (IS_ERR(gpriv->can_clk)) {
1777 err = PTR_ERR(gpriv->can_clk);
1778 dev_err(&pdev->dev,
1779 "cannot get canfd clock, error %d\n", err);
1780 goto fail_dev;
1781 }
1782 gpriv->fcan = RCANFD_CANFDCLK;
1783
1784 } else {
1785 gpriv->fcan = RCANFD_EXTCLK;
1786 }
1787 fcan_freq = clk_get_rate(gpriv->can_clk);
1788
1789 if (gpriv->fcan == RCANFD_CANFDCLK && gpriv->chip_id == RENESAS_RCAR_GEN3)
1790 /* CANFD clock is further divided by (1/2) within the IP */
1791 fcan_freq /= 2;
1792
1793 addr = devm_platform_ioremap_resource(pdev, 0);
1794 if (IS_ERR(addr)) {
1795 err = PTR_ERR(addr);
1796 goto fail_dev;
1797 }
1798 gpriv->base = addr;
1799
1800 /* Request IRQ that's common for both channels */
1801 if (gpriv->chip_id == RENESAS_RCAR_GEN3) {
1802 err = devm_request_irq(&pdev->dev, ch_irq,
1803 rcar_canfd_channel_interrupt, 0,
1804 "canfd.ch_int", gpriv);
1805 if (err) {
1806 dev_err(&pdev->dev, "devm_request_irq(%d) failed, error %d\n",
1807 ch_irq, err);
1808 goto fail_dev;
1809 }
1810
1811 err = devm_request_irq(&pdev->dev, g_irq,
1812 rcar_canfd_global_interrupt, 0,
1813 "canfd.g_int", gpriv);
1814 if (err) {
1815 dev_err(&pdev->dev, "devm_request_irq(%d) failed, error %d\n",
1816 g_irq, err);
1817 goto fail_dev;
1818 }
1819 } else {
1820 err = devm_request_irq(&pdev->dev, g_recc_irq,
1821 rcar_canfd_global_interrupt, 0,
1822 "canfd.g_recc", gpriv);
1823
1824 if (err) {
1825 dev_err(&pdev->dev, "devm_request_irq(%d) failed, error %d\n",
1826 g_recc_irq, err);
1827 goto fail_dev;
1828 }
1829
1830 err = devm_request_irq(&pdev->dev, g_err_irq,
1831 rcar_canfd_global_interrupt, 0,
1832 "canfd.g_err", gpriv);
1833 if (err) {
1834 dev_err(&pdev->dev, "devm_request_irq(%d) failed, error %d\n",
1835 g_err_irq, err);
1836 goto fail_dev;
1837 }
1838 }
1839
1840 err = reset_control_reset(gpriv->rstc1);
1841 if (err)
1842 goto fail_dev;
1843 err = reset_control_reset(gpriv->rstc2);
1844 if (err) {
1845 reset_control_assert(gpriv->rstc1);
1846 goto fail_dev;
1847 }
1848
1849 /* Enable peripheral clock for register access */
1850 err = clk_prepare_enable(gpriv->clkp);
1851 if (err) {
1852 dev_err(&pdev->dev,
1853 "failed to enable peripheral clock, error %d\n", err);
1854 goto fail_reset;
1855 }
1856
1857 err = rcar_canfd_reset_controller(gpriv);
1858 if (err) {
1859 dev_err(&pdev->dev, "reset controller failed\n");
1860 goto fail_clk;
1861 }
1862
1863 /* Controller in Global reset & Channel reset mode */
1864 rcar_canfd_configure_controller(gpriv);
1865
1866 /* Configure per channel attributes */
1867 for_each_set_bit(ch, &gpriv->channels_mask, RCANFD_NUM_CHANNELS) {
1868 /* Configure Channel's Rx fifo */
1869 rcar_canfd_configure_rx(gpriv, ch);
1870
1871 /* Configure Channel's Tx (Common) fifo */
1872 rcar_canfd_configure_tx(gpriv, ch);
1873
1874 /* Configure receive rules */
1875 rcar_canfd_configure_afl_rules(gpriv, ch);
1876 }
1877
1878 /* Configure common interrupts */
1879 rcar_canfd_enable_global_interrupts(gpriv);
1880
1881 /* Start Global operation mode */
1882 rcar_canfd_update_bit(gpriv->base, RCANFD_GCTR, RCANFD_GCTR_GMDC_MASK,
1883 RCANFD_GCTR_GMDC_GOPM);
1884
1885 /* Verify mode change */
1886 err = readl_poll_timeout((gpriv->base + RCANFD_GSTS), sts,
1887 !(sts & RCANFD_GSTS_GNOPM), 2, 500000);
1888 if (err) {
1889 dev_err(&pdev->dev, "global operational mode failed\n");
1890 goto fail_mode;
1891 }
1892
1893 for_each_set_bit(ch, &gpriv->channels_mask, RCANFD_NUM_CHANNELS) {
1894 err = rcar_canfd_channel_probe(gpriv, ch, fcan_freq);
1895 if (err)
1896 goto fail_channel;
1897 }
1898
1899 platform_set_drvdata(pdev, gpriv);
1900 dev_info(&pdev->dev, "global operational state (clk %d, fdmode %d)\n",
1901 gpriv->fcan, gpriv->fdmode);
1902 return 0;
1903
1904 fail_channel:
1905 for_each_set_bit(ch, &gpriv->channels_mask, RCANFD_NUM_CHANNELS)
1906 rcar_canfd_channel_remove(gpriv, ch);
1907 fail_mode:
1908 rcar_canfd_disable_global_interrupts(gpriv);
1909 fail_clk:
1910 clk_disable_unprepare(gpriv->clkp);
1911 fail_reset:
1912 reset_control_assert(gpriv->rstc1);
1913 reset_control_assert(gpriv->rstc2);
1914 fail_dev:
1915 return err;
1916 }
1917
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
On 25.07.2021 13:39:37, kernel test robot wrote: > Hi Lad, > > Thank you for the patch! Perhaps something to improve: > > [auto build test WARNING on renesas-devel/next] > [also build test WARNING on v5.14-rc2 next-20210723] > [cannot apply to mkl-can-next/testing robh/for-next] > [If your patch is applied to the wrong git tree, kindly drop us a note. > And when submitting patch, we suggest to use '--base' as documented in > https://git-scm.com/docs/git-format-patch] > > url: https://github.com/0day-ci/linux/commits/Lad-Prabhakar/Renesas-RZ-G2L-CANFD-support/20210722-035332 > base: https://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-devel.git next > config: arm64-randconfig-r031-20210723 (attached as .config) > compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 9625ca5b602616b2f5584e8a49ba93c52c141e40) > reproduce (this is a W=1 build): > wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross > chmod +x ~/bin/make.cross > # install arm64 cross compiling tool for clang build > # apt-get install binutils-aarch64-linux-gnu > # https://github.com/0day-ci/linux/commit/082d605e73c5922419a736aa9ecd3a76c0241bf7 > git remote add linux-review https://github.com/0day-ci/linux > git fetch --no-tags linux-review Lad-Prabhakar/Renesas-RZ-G2L-CANFD-support/20210722-035332 > git checkout 082d605e73c5922419a736aa9ecd3a76c0241bf7 > # save the attached .config to linux build tree > COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=arm64 > > If you fix the issue, kindly add following tag as appropriate > Reported-by: kernel test robot <lkp@intel.com> > > All warnings (new ones prefixed by >>): > > >> drivers/net/can/rcar/rcar_canfd.c:1699:12: warning: cast to smaller integer type 'enum rcanfd_chip_id' from 'const void *' [-Wvoid-pointer-to-enum-cast] > chip_id = (enum rcanfd_chip_id)of_device_get_match_data(&pdev->dev); > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > 1 warning generated. Seems we need the cast (uintptr_t), that I asked you to remove. Can you test if | chip_id = (enum rcanfd_chip_id)(uintptr_t)of_device_get_match_data(&pdev->dev); works? Marc -- Pengutronix e.K. | Marc Kleine-Budde | Embedded Linux | https://www.pengutronix.de | Vertretung West/Dortmund | Phone: +49-231-2826-924 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
Hi Marc, On Sun, Jul 25, 2021 at 11:46 AM Marc Kleine-Budde <mkl@pengutronix.de> wrote: > On 25.07.2021 13:39:37, kernel test robot wrote: > > [auto build test WARNING on renesas-devel/next] > > [also build test WARNING on v5.14-rc2 next-20210723] > > [cannot apply to mkl-can-next/testing robh/for-next] > > [If your patch is applied to the wrong git tree, kindly drop us a note. > > And when submitting patch, we suggest to use '--base' as documented in > > https://git-scm.com/docs/git-format-patch] > > > > url: https://github.com/0day-ci/linux/commits/Lad-Prabhakar/Renesas-RZ-G2L-CANFD-support/20210722-035332 > > base: https://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-devel.git next > > config: arm64-randconfig-r031-20210723 (attached as .config) > > compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 9625ca5b602616b2f5584e8a49ba93c52c141e40) > > reproduce (this is a W=1 build): > > wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross > > chmod +x ~/bin/make.cross > > # install arm64 cross compiling tool for clang build > > # apt-get install binutils-aarch64-linux-gnu > > # https://github.com/0day-ci/linux/commit/082d605e73c5922419a736aa9ecd3a76c0241bf7 > > git remote add linux-review https://github.com/0day-ci/linux > > git fetch --no-tags linux-review Lad-Prabhakar/Renesas-RZ-G2L-CANFD-support/20210722-035332 > > git checkout 082d605e73c5922419a736aa9ecd3a76c0241bf7 > > # save the attached .config to linux build tree > > COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=arm64 > > > > If you fix the issue, kindly add following tag as appropriate > > Reported-by: kernel test robot <lkp@intel.com> > > > > All warnings (new ones prefixed by >>): > > > > >> drivers/net/can/rcar/rcar_canfd.c:1699:12: warning: cast to smaller integer type 'enum rcanfd_chip_id' from 'const void *' [-Wvoid-pointer-to-enum-cast] > > chip_id = (enum rcanfd_chip_id)of_device_get_match_data(&pdev->dev); > > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > 1 warning generated. > > Seems we need the cast (uintptr_t), that I asked you to remove. Can you Bummer, I had seen your comment while reading email on my phone, but forgot to reply when I got back to my computer... > test if > > | chip_id = (enum rcanfd_chip_id)(uintptr_t)of_device_get_match_data(&pdev->dev); > > works? Just chip_id = (uintptr_t)of_device_get_match_data(&pdev->dev); should be fine. 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
Hi Prabhakar, On Wed, Jul 21, 2021 at 9:50 PM Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote: > CANFD block on RZ/G2L SoC is almost identical to one found on > R-Car Gen3 SoC's. On RZ/G2L SoC interrupt sources for each channel > are split into different sources and the IP doesn't divide (1/2) > CANFD clock within the IP. > > This patch adds compatible string for RZ/G2L family and registers > the irq handlers required for CANFD operation. IRQ numbers are now > fetched based on names instead of indices. For backward compatibility > on non RZ/G2L SoC's we fallback reading based on indices. > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> Thanks for the update! I think you misunderstood my comment on v1 about the interrupt handlers, cfr. below. > --- a/drivers/net/can/rcar/rcar_canfd.c > +++ b/drivers/net/can/rcar/rcar_canfd.c > @@ -1577,6 +1586,53 @@ 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) { > + char *irq_name; > + int err_irq; > + int tx_irq; > + > + err_irq = platform_get_irq_byname(pdev, ch == 0 ? "ch0_err" : "ch1_err"); > + if (err_irq < 0) { > + err = err_irq; > + goto fail; > + } > + > + tx_irq = platform_get_irq_byname(pdev, ch == 0 ? "ch0_trx" : "ch1_trx"); > + if (tx_irq < 0) { > + err = tx_irq; > + goto fail; > + } > + > + irq_name = devm_kasprintf(&pdev->dev, GFP_KERNEL, > + "canfd.ch%d_err", ch); > + if (!irq_name) { > + err = -ENOMEM; > + goto fail; > + } > + err = devm_request_irq(&pdev->dev, err_irq, > + rcar_canfd_channel_interrupt, 0, This is the same interrupt handler... > + irq_name, gpriv); > + if (err) { > + dev_err(&pdev->dev, "devm_request_irq CH Err(%d) failed, error %d\n", > + err_irq, err); > + goto fail; > + } > + irq_name = devm_kasprintf(&pdev->dev, GFP_KERNEL, > + "canfd.ch%d_trx", ch); > + if (!irq_name) { > + err = -ENOMEM; > + goto fail; > + } > + err = devm_request_irq(&pdev->dev, tx_irq, > + rcar_canfd_channel_interrupt, 0, ... as this one. > + irq_name, gpriv); > + if (err) { > + dev_err(&pdev->dev, "devm_request_irq Tx (%d) failed, error %d\n", > + tx_irq, err); > + goto fail; > + } > + } > + > if (gpriv->fdmode) { > priv->can.bittiming_const = &rcar_canfd_nom_bittiming_const; > priv->can.data_bittiming_const = > @@ -1711,20 +1798,51 @@ static int rcar_canfd_probe(struct platform_device *pdev) > gpriv->base = addr; > > /* Request IRQ that's common for both channels */ > - err = devm_request_irq(&pdev->dev, ch_irq, > - rcar_canfd_channel_interrupt, 0, > - "canfd.chn", gpriv); > - if (err) { > - dev_err(&pdev->dev, "devm_request_irq(%d) failed, error %d\n", > - ch_irq, err); > - goto fail_dev; > + if (gpriv->chip_id == RENESAS_RCAR_GEN3) { > + err = devm_request_irq(&pdev->dev, ch_irq, > + rcar_canfd_channel_interrupt, 0, > + "canfd.ch_int", gpriv); > + if (err) { > + dev_err(&pdev->dev, "devm_request_irq(%d) failed, error %d\n", > + ch_irq, err); > + goto fail_dev; > + } > + > + err = devm_request_irq(&pdev->dev, g_irq, > + rcar_canfd_global_interrupt, 0, > + "canfd.g_int", gpriv); > + if (err) { > + dev_err(&pdev->dev, "devm_request_irq(%d) failed, error %d\n", > + g_irq, err); > + goto fail_dev; > + } > + } else { > + err = devm_request_irq(&pdev->dev, g_recc_irq, > + rcar_canfd_global_interrupt, 0, This is the same interrupt handler... > + "canfd.g_recc", gpriv); > + > + if (err) { > + dev_err(&pdev->dev, "devm_request_irq(%d) failed, error %d\n", > + g_recc_irq, err); > + goto fail_dev; > + } > + > + err = devm_request_irq(&pdev->dev, g_err_irq, > + rcar_canfd_global_interrupt, 0, ... as this one. > + "canfd.g_err", gpriv); > + if (err) { > + dev_err(&pdev->dev, "devm_request_irq(%d) failed, error %d\n", > + g_err_irq, err); > + goto fail_dev; > + } > } > - err = devm_request_irq(&pdev->dev, g_irq, > - rcar_canfd_global_interrupt, 0, > - "canfd.gbl", gpriv); > + > + err = reset_control_reset(gpriv->rstc1); > + if (err) > + goto fail_dev; > + err = reset_control_reset(gpriv->rstc2); > if (err) { > - dev_err(&pdev->dev, "devm_request_irq(%d) failed, error %d\n", > - g_irq, err); > + reset_control_assert(gpriv->rstc1); > goto fail_dev; > } I did not object to having fine-grained interrupt handlers on RZ/G2L. I did object to duplicating code in global and fine-grained interrupt handlers. The trick to have both is to let the global interrupt handlers call (conditionally) into the fine-grained handlers. In pseudo-code: global_interrupt_handler() { if (...) fine_grained_handler1(); if (...) fine_grained_handler2(); ... } On R-Car Gen3, you register the global interrupt handlers, as before. On RZ/G2L, you register the fine-grained interrupt handlers instead. Gr{oetje,eeting}s, Geert
Hi Geert, Marc, On Mon, Jul 26, 2021 at 9:07 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > Hi Marc, > > On Sun, Jul 25, 2021 at 11:46 AM Marc Kleine-Budde <mkl@pengutronix.de> wrote: > > On 25.07.2021 13:39:37, kernel test robot wrote: > > > [auto build test WARNING on renesas-devel/next] > > > [also build test WARNING on v5.14-rc2 next-20210723] > > > [cannot apply to mkl-can-next/testing robh/for-next] > > > [If your patch is applied to the wrong git tree, kindly drop us a note. > > > And when submitting patch, we suggest to use '--base' as documented in > > > https://git-scm.com/docs/git-format-patch] > > > > > > url: https://github.com/0day-ci/linux/commits/Lad-Prabhakar/Renesas-RZ-G2L-CANFD-support/20210722-035332 > > > base: https://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-devel.git next > > > config: arm64-randconfig-r031-20210723 (attached as .config) > > > compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 9625ca5b602616b2f5584e8a49ba93c52c141e40) > > > reproduce (this is a W=1 build): > > > wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross > > > chmod +x ~/bin/make.cross > > > # install arm64 cross compiling tool for clang build > > > # apt-get install binutils-aarch64-linux-gnu > > > # https://github.com/0day-ci/linux/commit/082d605e73c5922419a736aa9ecd3a76c0241bf7 > > > git remote add linux-review https://github.com/0day-ci/linux > > > git fetch --no-tags linux-review Lad-Prabhakar/Renesas-RZ-G2L-CANFD-support/20210722-035332 > > > git checkout 082d605e73c5922419a736aa9ecd3a76c0241bf7 > > > # save the attached .config to linux build tree > > > COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=arm64 > > > > > > If you fix the issue, kindly add following tag as appropriate > > > Reported-by: kernel test robot <lkp@intel.com> > > > > > > All warnings (new ones prefixed by >>): > > > > > > >> drivers/net/can/rcar/rcar_canfd.c:1699:12: warning: cast to smaller integer type 'enum rcanfd_chip_id' from 'const void *' [-Wvoid-pointer-to-enum-cast] > > > chip_id = (enum rcanfd_chip_id)of_device_get_match_data(&pdev->dev); > > > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > > 1 warning generated. > > > > Seems we need the cast (uintptr_t), that I asked you to remove. Can you > > Bummer, I had seen your comment while reading email on my phone, > but forgot to reply when I got back to my computer... > > > test if > > > > | chip_id = (enum rcanfd_chip_id)(uintptr_t)of_device_get_match_data(&pdev->dev); > > > > works? > > Just > > chip_id = (uintptr_t)of_device_get_match_data(&pdev->dev); > > should be fine. > Above works, cast is not required. Cheers, Prabhakar > 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
Hi Geert, Thank you for the review. On Mon, Jul 26, 2021 at 10:53 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > Hi Prabhakar, > > On Wed, Jul 21, 2021 at 9:50 PM Lad Prabhakar > <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote: > > CANFD block on RZ/G2L SoC is almost identical to one found on > > R-Car Gen3 SoC's. On RZ/G2L SoC interrupt sources for each channel > > are split into different sources and the IP doesn't divide (1/2) > > CANFD clock within the IP. > > > > This patch adds compatible string for RZ/G2L family and registers > > the irq handlers required for CANFD operation. IRQ numbers are now > > fetched based on names instead of indices. For backward compatibility > > on non RZ/G2L SoC's we fallback reading based on indices. > > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > Thanks for the update! > > I think you misunderstood my comment on v1 about the interrupt > handlers, cfr. below. > Argh my bad I took it the other way round! > > --- a/drivers/net/can/rcar/rcar_canfd.c > > +++ b/drivers/net/can/rcar/rcar_canfd.c > > > @@ -1577,6 +1586,53 @@ 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) { > > + char *irq_name; > > + int err_irq; > > + int tx_irq; > > + > > + err_irq = platform_get_irq_byname(pdev, ch == 0 ? "ch0_err" : "ch1_err"); > > + if (err_irq < 0) { > > + err = err_irq; > > + goto fail; > > + } > > + > > + tx_irq = platform_get_irq_byname(pdev, ch == 0 ? "ch0_trx" : "ch1_trx"); > > + if (tx_irq < 0) { > > + err = tx_irq; > > + goto fail; > > + } > > + > > + irq_name = devm_kasprintf(&pdev->dev, GFP_KERNEL, > > + "canfd.ch%d_err", ch); > > + if (!irq_name) { > > + err = -ENOMEM; > > + goto fail; > > + } > > + err = devm_request_irq(&pdev->dev, err_irq, > > + rcar_canfd_channel_interrupt, 0, > > This is the same interrupt handler... > > > + irq_name, gpriv); > > + if (err) { > > + dev_err(&pdev->dev, "devm_request_irq CH Err(%d) failed, error %d\n", > > + err_irq, err); > > + goto fail; > > + } > > + irq_name = devm_kasprintf(&pdev->dev, GFP_KERNEL, > > + "canfd.ch%d_trx", ch); > > + if (!irq_name) { > > + err = -ENOMEM; > > + goto fail; > > + } > > + err = devm_request_irq(&pdev->dev, tx_irq, > > + rcar_canfd_channel_interrupt, 0, > > ... as this one. > > > + irq_name, gpriv); > > + if (err) { > > + dev_err(&pdev->dev, "devm_request_irq Tx (%d) failed, error %d\n", > > + tx_irq, err); > > + goto fail; > > + } > > + } > > + > > if (gpriv->fdmode) { > > priv->can.bittiming_const = &rcar_canfd_nom_bittiming_const; > > priv->can.data_bittiming_const = > > > @@ -1711,20 +1798,51 @@ static int rcar_canfd_probe(struct platform_device *pdev) > > gpriv->base = addr; > > > > /* Request IRQ that's common for both channels */ > > - err = devm_request_irq(&pdev->dev, ch_irq, > > - rcar_canfd_channel_interrupt, 0, > > - "canfd.chn", gpriv); > > - if (err) { > > - dev_err(&pdev->dev, "devm_request_irq(%d) failed, error %d\n", > > - ch_irq, err); > > - goto fail_dev; > > + if (gpriv->chip_id == RENESAS_RCAR_GEN3) { > > + err = devm_request_irq(&pdev->dev, ch_irq, > > + rcar_canfd_channel_interrupt, 0, > > + "canfd.ch_int", gpriv); > > + if (err) { > > + dev_err(&pdev->dev, "devm_request_irq(%d) failed, error %d\n", > > + ch_irq, err); > > + goto fail_dev; > > + } > > + > > + err = devm_request_irq(&pdev->dev, g_irq, > > + rcar_canfd_global_interrupt, 0, > > + "canfd.g_int", gpriv); > > + if (err) { > > + dev_err(&pdev->dev, "devm_request_irq(%d) failed, error %d\n", > > + g_irq, err); > > + goto fail_dev; > > + } > > + } else { > > + err = devm_request_irq(&pdev->dev, g_recc_irq, > > + rcar_canfd_global_interrupt, 0, > > This is the same interrupt handler... > > > + "canfd.g_recc", gpriv); > > + > > + if (err) { > > + dev_err(&pdev->dev, "devm_request_irq(%d) failed, error %d\n", > > + g_recc_irq, err); > > + goto fail_dev; > > + } > > + > > + err = devm_request_irq(&pdev->dev, g_err_irq, > > + rcar_canfd_global_interrupt, 0, > > ... as this one. > > > + "canfd.g_err", gpriv); > > + if (err) { > > + dev_err(&pdev->dev, "devm_request_irq(%d) failed, error %d\n", > > + g_err_irq, err); > > + goto fail_dev; > > + } > > } > > - err = devm_request_irq(&pdev->dev, g_irq, > > - rcar_canfd_global_interrupt, 0, > > - "canfd.gbl", gpriv); > > + > > + err = reset_control_reset(gpriv->rstc1); > > + if (err) > > + goto fail_dev; > > + err = reset_control_reset(gpriv->rstc2); > > if (err) { > > - dev_err(&pdev->dev, "devm_request_irq(%d) failed, error %d\n", > > - g_irq, err); > > + reset_control_assert(gpriv->rstc1); > > goto fail_dev; > > } > > I did not object to having fine-grained interrupt handlers on RZ/G2L. > I did object to duplicating code in global and fine-grained interrupt > handlers. > > The trick to have both is to let the global interrupt handlers call > (conditionally) into the fine-grained handlers. In pseudo-code: > > global_interrupt_handler() > { > if (...) > fine_grained_handler1(); > > if (...) > fine_grained_handler2(); > ... > } > > On R-Car Gen3, you register the global interrupt handlers, as before. > On RZ/G2L, you register the fine-grained interrupt handlers instead. > Agreed will re-spin with the fine-grained version tomorrow. Cheers, Prabhakar > 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
diff --git a/drivers/net/can/rcar/rcar_canfd.c b/drivers/net/can/rcar/rcar_canfd.c index 311e6ca3bdc4..04747573fc48 100644 --- a/drivers/net/can/rcar/rcar_canfd.c +++ b/drivers/net/can/rcar/rcar_canfd.c @@ -37,9 +37,15 @@ #include <linux/bitmap.h> #include <linux/bitops.h> #include <linux/iopoll.h> +#include <linux/reset.h> #define RCANFD_DRV_NAME "rcar_canfd" +enum rcanfd_chip_id { + RENESAS_RCAR_GEN3 = 0, + RENESAS_RZG2L, +}; + /* Global register bits */ /* RSCFDnCFDGRMCFG */ @@ -513,6 +519,9 @@ struct rcar_canfd_global { enum rcar_canfd_fcanclk fcan; /* CANFD or Ext clock */ unsigned long channels_mask; /* Enabled channels mask */ bool fdmode; /* CAN FD or Classical CAN only mode */ + struct reset_control *rstc1; + struct reset_control *rstc2; + enum rcanfd_chip_id chip_id; }; /* CAN FD mode nominal rate constants */ @@ -1577,6 +1586,53 @@ 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) { + char *irq_name; + int err_irq; + int tx_irq; + + err_irq = platform_get_irq_byname(pdev, ch == 0 ? "ch0_err" : "ch1_err"); + if (err_irq < 0) { + err = err_irq; + goto fail; + } + + tx_irq = platform_get_irq_byname(pdev, ch == 0 ? "ch0_trx" : "ch1_trx"); + if (tx_irq < 0) { + err = tx_irq; + goto fail; + } + + irq_name = devm_kasprintf(&pdev->dev, GFP_KERNEL, + "canfd.ch%d_err", ch); + if (!irq_name) { + err = -ENOMEM; + goto fail; + } + err = devm_request_irq(&pdev->dev, err_irq, + rcar_canfd_channel_interrupt, 0, + irq_name, gpriv); + if (err) { + dev_err(&pdev->dev, "devm_request_irq CH Err(%d) failed, error %d\n", + err_irq, err); + goto fail; + } + irq_name = devm_kasprintf(&pdev->dev, GFP_KERNEL, + "canfd.ch%d_trx", ch); + if (!irq_name) { + err = -ENOMEM; + goto fail; + } + err = devm_request_irq(&pdev->dev, tx_irq, + rcar_canfd_channel_interrupt, 0, + irq_name, gpriv); + if (err) { + dev_err(&pdev->dev, "devm_request_irq Tx (%d) failed, error %d\n", + tx_irq, err); + goto fail; + } + } + if (gpriv->fdmode) { priv->can.bittiming_const = &rcar_canfd_nom_bittiming_const; priv->can.data_bittiming_const = @@ -1636,7 +1692,11 @@ static int rcar_canfd_probe(struct platform_device *pdev) struct device_node *of_child; unsigned long channels_mask = 0; 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; + + chip_id = (enum rcanfd_chip_id)of_device_get_match_data(&pdev->dev); if (of_property_read_bool(pdev->dev.of_node, "renesas,no-can-fd")) fdmode = false; /* Classical CAN only mode */ @@ -1649,16 +1709,30 @@ static int rcar_canfd_probe(struct platform_device *pdev) if (of_child && of_device_is_available(of_child)) channels_mask |= BIT(1); /* Channel 1 */ - ch_irq = platform_get_irq(pdev, 0); - if (ch_irq < 0) { - err = ch_irq; - goto fail_dev; - } + if (chip_id == RENESAS_RCAR_GEN3) { + ch_irq = platform_get_irq_byname_optional(pdev, "ch_int"); + if (ch_irq < 0) { + /* For backward compatibility get irq by index */ + ch_irq = platform_get_irq(pdev, 0); + if (ch_irq < 0) + return ch_irq; + } - g_irq = platform_get_irq(pdev, 1); - if (g_irq < 0) { - err = g_irq; - goto fail_dev; + g_irq = platform_get_irq_byname_optional(pdev, "g_int"); + if (g_irq < 0) { + /* For backward compatibility get irq by index */ + g_irq = platform_get_irq(pdev, 1); + if (g_irq < 0) + return g_irq; + } + } else { + g_err_irq = platform_get_irq_byname(pdev, "g_err"); + if (g_err_irq < 0) + return g_err_irq; + + g_recc_irq = platform_get_irq_byname(pdev, "g_recc"); + if (g_recc_irq < 0) + return g_recc_irq; } /* Global controller context */ @@ -1670,6 +1744,19 @@ 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; + + if (gpriv->chip_id == RENESAS_RZG2L) { + gpriv->rstc1 = devm_reset_control_get_exclusive(&pdev->dev, "rstp_n"); + if (IS_ERR(gpriv->rstc1)) + return dev_err_probe(&pdev->dev, PTR_ERR(gpriv->rstc1), + "failed to get rstp_n\n"); + + gpriv->rstc2 = devm_reset_control_get_exclusive(&pdev->dev, "rstc_n"); + if (IS_ERR(gpriv->rstc2)) + return dev_err_probe(&pdev->dev, PTR_ERR(gpriv->rstc2), + "failed to get rstc_n\n"); + } /* Peripheral clock */ gpriv->clkp = devm_clk_get(&pdev->dev, "fck"); @@ -1699,7 +1786,7 @@ static int rcar_canfd_probe(struct platform_device *pdev) } fcan_freq = clk_get_rate(gpriv->can_clk); - if (gpriv->fcan == RCANFD_CANFDCLK) + if (gpriv->fcan == RCANFD_CANFDCLK && gpriv->chip_id == RENESAS_RCAR_GEN3) /* CANFD clock is further divided by (1/2) within the IP */ fcan_freq /= 2; @@ -1711,20 +1798,51 @@ static int rcar_canfd_probe(struct platform_device *pdev) gpriv->base = addr; /* Request IRQ that's common for both channels */ - err = devm_request_irq(&pdev->dev, ch_irq, - rcar_canfd_channel_interrupt, 0, - "canfd.chn", gpriv); - if (err) { - dev_err(&pdev->dev, "devm_request_irq(%d) failed, error %d\n", - ch_irq, err); - goto fail_dev; + if (gpriv->chip_id == RENESAS_RCAR_GEN3) { + err = devm_request_irq(&pdev->dev, ch_irq, + rcar_canfd_channel_interrupt, 0, + "canfd.ch_int", gpriv); + if (err) { + dev_err(&pdev->dev, "devm_request_irq(%d) failed, error %d\n", + ch_irq, err); + goto fail_dev; + } + + err = devm_request_irq(&pdev->dev, g_irq, + rcar_canfd_global_interrupt, 0, + "canfd.g_int", gpriv); + if (err) { + dev_err(&pdev->dev, "devm_request_irq(%d) failed, error %d\n", + g_irq, err); + goto fail_dev; + } + } else { + err = devm_request_irq(&pdev->dev, g_recc_irq, + rcar_canfd_global_interrupt, 0, + "canfd.g_recc", gpriv); + + if (err) { + dev_err(&pdev->dev, "devm_request_irq(%d) failed, error %d\n", + g_recc_irq, err); + goto fail_dev; + } + + err = devm_request_irq(&pdev->dev, g_err_irq, + rcar_canfd_global_interrupt, 0, + "canfd.g_err", gpriv); + if (err) { + dev_err(&pdev->dev, "devm_request_irq(%d) failed, error %d\n", + g_err_irq, err); + goto fail_dev; + } } - err = devm_request_irq(&pdev->dev, g_irq, - rcar_canfd_global_interrupt, 0, - "canfd.gbl", gpriv); + + err = reset_control_reset(gpriv->rstc1); + if (err) + goto fail_dev; + err = reset_control_reset(gpriv->rstc2); if (err) { - dev_err(&pdev->dev, "devm_request_irq(%d) failed, error %d\n", - g_irq, err); + reset_control_assert(gpriv->rstc1); goto fail_dev; } @@ -1733,7 +1851,7 @@ static int rcar_canfd_probe(struct platform_device *pdev) if (err) { dev_err(&pdev->dev, "failed to enable peripheral clock, error %d\n", err); - goto fail_dev; + goto fail_reset; } err = rcar_canfd_reset_controller(gpriv); @@ -1790,6 +1908,9 @@ static int rcar_canfd_probe(struct platform_device *pdev) rcar_canfd_disable_global_interrupts(gpriv); fail_clk: clk_disable_unprepare(gpriv->clkp); +fail_reset: + reset_control_assert(gpriv->rstc1); + reset_control_assert(gpriv->rstc2); fail_dev: return err; } @@ -1810,6 +1931,9 @@ static int rcar_canfd_remove(struct platform_device *pdev) /* Enter global sleep mode */ rcar_canfd_set_bit(gpriv->base, RCANFD_GCTR, RCANFD_GCTR_GSLPR); clk_disable_unprepare(gpriv->clkp); + reset_control_assert(gpriv->rstc1); + reset_control_assert(gpriv->rstc2); + return 0; } @@ -1827,7 +1951,8 @@ static SIMPLE_DEV_PM_OPS(rcar_canfd_pm_ops, rcar_canfd_suspend, rcar_canfd_resume); static const struct of_device_id rcar_canfd_of_table[] = { - { .compatible = "renesas,rcar-gen3-canfd" }, + { .compatible = "renesas,rcar-gen3-canfd", .data = (void *)RENESAS_RCAR_GEN3 }, + { .compatible = "renesas,rzg2l-canfd", .data = (void *)RENESAS_RZG2L }, { } };