On Fri, Feb 14, 2020 at 11:02:46AM +0100, Michael Walle wrote: > > Hi Joakim, Hi Shawn, > > > Am 2020-02-14 10:56, schrieb Joakim Zhang: > > > -----Original Message----- > > > From: Michael Walle <michael@xxxxxxxx> > > > Sent: 2020年2月14日 17:33 > > > To: Joakim Zhang <qiangqing.zhang@xxxxxxx> > > > Cc: Marc Kleine-Budde <mkl@xxxxxxxxxxxxxx>; wg@xxxxxxxxxxxxxx; > > > netdev@xxxxxxxxxxxxxxx; linux-can@xxxxxxxxxxxxxxx; Pankaj Bansal > > > <pankaj.bansal@xxxxxxx>; Shawn Guo <shawnguo@xxxxxxxxxx> > > > Subject: Re: [PATCH 0/8] can: flexcan: add CAN FD support for NXP > > > Flexcan > > > > > > Am 2020-02-14 10:18, schrieb Joakim Zhang: > > > > Best Regards, > > > > Joakim Zhang > > > > > > > >> -----Original Message----- > > > >> From: Michael Walle <michael@xxxxxxxx> > > > >> Sent: 2020年2月14日 16:43 > > > >> To: Joakim Zhang <qiangqing.zhang@xxxxxxx> > > > >> Cc: Marc Kleine-Budde <mkl@xxxxxxxxxxxxxx>; wg@xxxxxxxxxxxxxx; > > > >> netdev@xxxxxxxxxxxxxxx; linux-can@xxxxxxxxxxxxxxx; Pankaj Bansal > > > >> <pankaj.bansal@xxxxxxx> > > > >> Subject: Re: [PATCH 0/8] can: flexcan: add CAN FD support for NXP > > > >> Flexcan > > > >> > > > >> Hi Joakim, > > > >> > > > >> Am 2020-02-14 02:55, schrieb Joakim Zhang: > > > >> > Hi Michal, > > > >> > > > > >> >> -----Original Message----- > > > >> >> From: Michael Walle <michael@xxxxxxxx> > > > >> >> Sent: 2020年2月14日 3:20 > > > >> >> To: Marc Kleine-Budde <mkl@xxxxxxxxxxxxxx> > > > >> >> Cc: Joakim Zhang <qiangqing.zhang@xxxxxxx>; wg@xxxxxxxxxxxxxx; > > > >> >> netdev@xxxxxxxxxxxxxxx; linux-can@xxxxxxxxxxxxxxx; Pankaj Bansal > > > >> >> <pankaj.bansal@xxxxxxx>; Michael Walle <michael@xxxxxxxx> > > > >> >> Subject: Re: [PATCH 0/8] can: flexcan: add CAN FD support for NXP > > > >> >> Flexcan > > > >> >> > > > >> >> Hi, > > > >> >> > > > >> >> >>> Are you prepared to add back these patches as they are > > > >> >> >>> necessary for Flexcan CAN FD? And this Flexcan CAN FD patch > > > >> >> >>> set is based on these patches. > > > >> >> >> > > > >> >> >> Yes, these patches will be added back. > > > >> >> > > > > >> >> >I've cleaned up the first patch a bit, and pushed everything to > > > >> >> >the testing branch. Can you give it a test. > > > >> >> > > > >> >> What happend to that branch? FWIW I've just tried the patches on a > > > >> >> custom board with a LS1028A SoC. Both CAN and CAN-FD are working. > > > >> >> I've tested against a Peaktech USB CAN adapter. I'd love to see > > > >> >> these patches upstream, because our board also offers CAN and > > > >> >> basic support for it just made it upstream [1]. > > > >> > The FlexCAN CAN FD related patches have stayed in > > > >> > linux-can-next/flexcan branch for a long time, I still don't know > > > >> > why Marc doesn't merge them into Linux mainline. > > > >> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit. > > > >> > > > > >> > > > kernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Fmkl%2Flinux-can-next. > > > >> g > > > >> > > > > >> > > > it%2Ftree%2F%3Fh%3Dflexcan&data=02%7C01%7Cqiangqing.zhang%40n > > > >> xp.co > > > >> > > > > >> > > > m%7C94dca4472a584410b3b908d7b129db27%7C686ea1d3bc2b4c6fa92cd99c > > > >> 5c30163 > > > >> > > > > >> > > > 5%7C0%7C0%7C637172665642079192&sdata=77tG6VuQCi%2FZXBKb23 > > > >> 8%2FdNSV3 > > > >> > NUIFrM5Y0e9yj0J3os%3D&reserved=0 > > > >> > Also must hope that this patch set can be upstreamed soon. :-) > > > >> > > > >> I've took them from this branch and applied them to the latest linux > > > >> master. > > > >> > > > >> Thus, > > > >> > > > >> Tested-by: Michael Walle <michael@xxxxxxxx> > > > >> > > > >> > > > >> >> If these patches are upstream, only the device tree nodes seems to > > > >> >> be missing. > > > >> >> I don't know what has happened to [2]. But the patch doesn't seem > > > >> >> to be necessary. > > > >> > Yes, this patch is unnecessary. I have NACKed this patch for that, > > > >> > according to FlexCAN Integrated Guide, CTRL1[CLKSRC]=0 select > > > >> > oscillator clock and CTRL1[CLKSRC]=1 select peripheral clock. > > > >> > But it is actually decided by SoC integration, for i.MX, the design > > > >> > is different. > > > >> > > > >> ok thanks for clarifying. > > > >> > > > >> > I have not upstream i.MX FlexCAN device tree nodes, since it's > > > >> > dependency have not upstreamed yet. > > > >> > > > > >> >> Pankaj already send a patch to add the device node to the LS1028A [3]. > > > >> >> Thats basically the same I've used, only that mine didn't had the > > > >> >> "fsl,ls1028ar1-flexcan" compatiblity string, but only the > > > >> >> "lx2160ar1-flexcan" > > > >> >> which is the correct way to use it, right? > > > >> > You can see below table from FlexCAN driver, "fsl,lx2160ar1-flexcan" > > > >> > supports CAN FD, you can use this compatible string. > > > >> > > > >> correct. I've already a patch that does exactly this ;) Who would > > > >> take the patch for adding the LS1028A can device tree nodes to > > > >> ls1028a.dtsi? You or Shawn Guo? > > > > Sorry, I missed the link[3], we usually write it this way: > > > > compatible = "fsl,ls1028ar1-flexcan","fsl,lx2160ar1-flexcan"; > > > > Please send patch to Shawn Guo, he will review the device tree. > > > > > > As far as I know, there should be no undocumented binding. Eg. the > > > ls1028ar1-flexcan is neither in the source nor in the device tree > > > binding > > > documentation, thus wouldn't be accepted. > > > > > > Thus either there should be another ls1028ar1-flexcan in the > > > flexcan_of_match > > > table and the node should only contain that string or the node > > > should only > > > contain fsl,lx2160ar1-flexcan. Is there any advantage of the first > > > option? > > From the FlexCAN > > binding(Documentation/devicetree/bindings/net/can/fsl-flexcan.txt) > > - compatible : Should be "fsl,<processor>-flexcan" > > > > An implementation should also claim any of the following compatibles > > that it is fully backwards compatible with: > > > > - fsl,p1010-flexcan > > > > You also can check imx6ul.dtsi imx7s.dtsi etc. > > > > Sorry :-(, I also don't know the advantage, it's just that we're used > > to writing it that way. You can check nodes of other devices. > > It's unnecessary to add compatible string for each SoCs since they may > > share the same IP. And dts had batter have a SoC specific compatible > > string. It's just my understanding. > > Ah thanks. So Pankaj's patch [1] seems to be correct (at least according > to the description in the device tree documentation). > > Shawn, whats your opinion? My opinion is that all compatibles should be defined explicitly in bindings doc. In above example, the possible values of <processor> should be given. This must be done anyway, as we are moving to json-schema bindings. Shawn
