| Message ID | 20240222181324.28242-3-fancer.lancer@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | EDAC/mc/synopsys: Various fixes and cleanups | expand |
On Thu, Feb 22, 2024 at 09:12:47PM +0300, Serge Semin wrote: > First of all the enum dev_type constants describe the memory DRAM chips > used at the stick, not the entire DQ-bus width (see the enumeration kdoc Which kdoc? The kernel doc above enum dev_type in include/linux/edac.h? In any case, you need to be precise pls. > for details). So what is returned from the zynqmp_get_dtype() function and > then specified to the dimm_info->dtype field is definitely incorrect. Whoops, you lost me here. Why is it incorrect? You want "zynqmp_get_dtype - Return the controller memory width." to return the memory width supported by the controller? dimm->dtype = p_data->get_dtype(priv->baseaddr); enum dev_type dtype; /* memory device type */ Yeah, no, that function returns the DIMM device type. /me looks at the code. Aha, so you mean the device width should be determined from that DDRC_MSTR_CFG* thing. > Secondly the DRAM chips type has nothing to do with the data bus width > specified in the MSTR.data_bus_width CSR field. That CSR field just > determines the part of the whole DQ-bus currently used to access the data > from the all DRAM memory chips. So it doesn't indicate the individual > chips type. Thirdly the DRAM chips type can be determined only in case of > the DDR4 protocol by means of the MSTR.device_config field state (it is Hold on, this driver runs on all kinds of hardware I presume. Are you thinking about older ones which don't do DDR4? Or does that thing do DDR4 only? > supposed to be set by the system firmware). Finally the DW uMCTL2 DDRC ECC > capability doesn't depend on the memory chips type. Moreover it doesn't > depend on the utilized data bus width in runtime either. The IP-core > reference manual says in [1,2] that the ECC support can't be enabled > during the IP-core synthesizes for the DRAM data bus widths other than 16, This sentence is missing something. > 32 or 64. At the same time the bus width mode (MSTR.data_bus_width) > doesn't change the ECC feature availability. Thus it was wrong to > determine the ECC state with respect to the DQ-bus width mode. You need to split your paragraphs with newlines to help readability. Right now it is a blob of hard to parse text. For example, when you have to write "Secondly, " that's your split right there. "Thirdly," is your next newline. And so on. > Fix all of the mistakes described above in the zynqmp_get_dtype() and > zynqmp_get_ecc_state() methods: specify actual DRAM chips data width only > for the DDR4 protocol and return that it's UNKNOWN in the rest of the > cases; What are the rest of the cases and why is it ok to return UNKNOWN all of a sudden? IOW, how was the old code even tested?! > determine ECC availability by the ECCCFG0.ecc_mode field state > only (that field can't be modified anyway if the IP-core was synthesized > with no ECC support). > > [1] DesignWare® Cores Enhanced Universal DDR Memory Controller (uMCTL2) > Databook, Version 3.91a, October 2020, p. 421. > [2] DesignWare® Cores Enhanced Universal DDR Memory Controller (uMCTL2) > Databook, Version 3.91a, October 2020, p. 633. Can those be freely accessed? If not, you should say so. > Fixes: b500b4a029d5 ("EDAC, synopsys: Add ECC support for ZynqMP DDR controller") So this commit is in 4.20. Does that mean that this fix needs to get backported to all stable kernels? Have you tested this on all hw this driver supports and made sure no regressions are introduced? Thx.
On Tue, Jun 04, 2024 at 08:38:15PM +0200, Borislav Petkov wrote: > On Thu, Feb 22, 2024 at 09:12:47PM +0300, Serge Semin wrote: > > First of all the enum dev_type constants describe the memory DRAM chips > > used at the stick, not the entire DQ-bus width (see the enumeration kdoc > > Which kdoc? > > The kernel doc above enum dev_type in include/linux/edac.h? Right. > > In any case, you need to be precise pls. > > > for details). So what is returned from the zynqmp_get_dtype() function and > > then specified to the dimm_info->dtype field is definitely incorrect. > > Whoops, you lost me here. Why is it incorrect? As I said because dev_type is the memory DRAM chips type (individual DRAM chip data bus width), and not the entire DQ-bus width or its currently active part. Even from that perspective the function name and the subsequent return value utilization is incorrect. Imagine the Xilinx ZynqMP has the 64-bit DQ-bus width. Having (MSTR.data_bus_width == DDRCTL_EWDTH_16) means that only a quarter of the bus will be utilized to get data from the DRAMs. So all the connected DRAM chip(s) data buses are _somehow_ distributed along the 16 bits of the DRAM controller DQ-bus. It can be a single DRAM chip with 16-bit DQ-bus, or two x8 DRAM chips, or four x4 DRAM chips, etc. > > You want > > "zynqmp_get_dtype - Return the controller memory width." > > to return the memory width supported by the controller? > > dimm->dtype = p_data->get_dtype(priv->baseaddr); > > enum dev_type dtype; /* memory device type */ > > Yeah, no, that function returns the DIMM device type. > > /me looks at the code. > > Aha, so you mean the device width should be determined from that > DDRC_MSTR_CFG* thing. That's what is said in "Secondly" and "Thirdly". > > > Secondly the DRAM chips type has nothing to do with the data bus width > > specified in the MSTR.data_bus_width CSR field. That CSR field just > > determines the part of the whole DQ-bus currently used to access the data > > from the all DRAM memory chips. So it doesn't indicate the individual > > chips type. Thirdly the DRAM chips type can be determined only in case of > > the DDR4 protocol by means of the MSTR.device_config field state (it is > > Hold on, this driver runs on all kinds of hardware I presume. Are you > thinking about older ones which don't do DDR4? > > Or does that thing do DDR4 only? First of all, not that much of the kinds. Just Xilinx ZynqMP DDRC (based on the DW uMCTL 2.40a IP-core) and some version of DW uMCTL 3.80a being possessed by Dinh Nguyen and, by a lucky coincident, turned to be mainly compatibly with the Xilinx ZynqMP DDR controller. Secondly I've checked that part on all the DW uMCTL2 databooks I've got (I've got lots: versions 1.x, 2.x and 3.x). DW uMCTL2 v1.x doesn't support DDR4. DW uMCTL2 v2.x and v3.x IP-cores do support DDR4 but work as I described: the only way to determine the DRAM chips type is to use the MSTR.device_config field content and for DDR4 only. MSTR.data_bus_width field has nothing to do with that. > > > supposed to be set by the system firmware). > > Finally the DW uMCTL2 DDRC ECC > > capability doesn't depend on the memory chips type. Moreover it doesn't > > depend on the utilized data bus width in runtime either. The IP-core > > reference manual says in [1,2] that the ECC support can't be enabled > > during the IP-core synthesizes for the DRAM data bus widths other than 16, > > This sentence is missing something. > > > 32 or 64. At the same time the bus width mode (MSTR.data_bus_width) > > doesn't change the ECC feature availability. Thus it was wrong to > > determine the ECC state with respect to the DQ-bus width mode. Sorry, but this part doesn't miss anything. It merely says that neither memory DRAM chips type nor MSTR.data_bus_width value could be utilized to determine the ECC support. According to the databooks the ECC support can be available on the IP-cores which _full_ DQ-bus width is 16, 32 or 64. MSTR.data_bus_width, selecting the active part of the full DQ-bus, doesn't change the ECC feature availability in anyway. From that perspective the zynqmp_get_dtype() utilization in zynqmp_get_ecc_state() has also been incorrect. > > You need to split your paragraphs with newlines to help readability. > Right now it is a blob of hard to parse text. For example, when you have > to write "Secondly, " that's your split right there. "Thirdly," is your > next newline. And so on. Ok. > > > Fix all of the mistakes described above in the zynqmp_get_dtype() and > > zynqmp_get_ecc_state() methods: specify actual DRAM chips data width only > > for the DDR4 protocol and return that it's UNKNOWN in the rest of the > > cases; > > What are the rest of the cases and why is it ok to return UNKNOWN all of > a sudden? IOW, how was the old code even tested?! First of all, MSTR.data_bus_width field can have only one of the next three values: 0x1, 0x2 and 0x3. All of them are handled in zynqmp_get_dtype(). So in the current (incorrect) implementation it will never return DEV_UNKNOWN. Secondly, dimm->dtype isn't utilized for something significant in the EDAC subsystem, but is just exposed to the user-space via the dev_type sysfs node. So based on that my bet is that since the incorrect code didn't affect the main driver functionality and since the dimm->dtype is just exposed to user-space, the bug has been living just fine unnoticed up until I started digging into the original DW uMCTL2 HW-manuals, started studying the driver code, and decided to convert the driver to supporting generic version of the DW uMCTL2 controller (not only the Xilinx version of it). That's what this series and the next two ones are about - about converting the driver to supporting truly generic DW uMCTL controllers. > > > determine ECC availability by the ECCCFG0.ecc_mode field state > > only (that field can't be modified anyway if the IP-core was synthesized > > with no ECC support). > > > > [1] DesignWare® Cores Enhanced Universal DDR Memory Controller (uMCTL2) > > Databook, Version 3.91a, October 2020, p. 421. > > [2] DesignWare® Cores Enhanced Universal DDR Memory Controller (uMCTL2) > > Databook, Version 3.91a, October 2020, p. 633. > > Can those be freely accessed? > > If not, you should say so. No, they can't be. > > > Fixes: b500b4a029d5 ("EDAC, synopsys: Add ECC support for ZynqMP DDR controller") > > So this commit is in 4.20. > > Does that mean that this fix needs to get backported to all stable > kernels? It's up to the stable maintainers to decide. > > Have you tested this on all hw this driver supports and made sure no > regressions are introduced? I've tested it on the devices with DW uMCTL 2.51a + DDR3 memory and DW uMCTL 3.10a + DDR4 memory. I am sure this will work for Xilinx ZynqMP too, especially seeing we've already got the Shubhrajyoti Datta Rb tag: https://lore.kernel.org/linux-edac/CAKfKVtErVuCM+pa1e7Lwt0DUU-t-U0eNRnZSw39pfsZ8gv8QZQ@mail.gmail.com/ -Serge(y) > > Thx. > > -- > Regards/Gruss, > Boris. > > https://people.kernel.org/tglx/notes-about-netiquette
On Wed, Jun 05, 2024 at 01:11:27AM +0300, Serge Semin wrote: > As I said because dev_type is the memory DRAM chips type (individual > DRAM chip data bus width), and not the entire DQ-bus width or its > currently active part. Even from that perspective the function name > and the subsequent return value utilization is incorrect. Well, maybe the author misunderstood it but the result of this goes to sysfs: dimm->dtype = p_data->get_dtype(priv->baseaddr); which is in Documentation/ABI/testing/sysfs-devices-edac: What: /sys/devices/system/edac/mc/mc*/(dimm|rank)*/dimm_dev_type Date: April 2012 Contact: Mauro Carvalho Chehab <mchehab+samsung@kernel.org> linux-edac@vger.kernel.org Description: This attribute file will display what type of DRAM device is being utilized on this DIMM (x1, x2, x4, x8, ...). So you'd need to fix the comment above zynqmp_get_dtype() or I can do so too while applying. > First of all, not that much of the kinds. What does that mean: "not that much of the kinds"? > Just Xilinx ZynqMP DDRC (based on the DW uMCTL 2.40a IP-core) and some > version of DW uMCTL 3.80a being possessed by Dinh Nguyen and, by > a lucky coincident, turned to be mainly compatibly with the Xilinx > ZynqMP DDR controller. Then Dinh better holler here what the story is. > > > 32 or 64. At the same time the bus width mode (MSTR.data_bus_width) > > > doesn't change the ECC feature availability. Thus it was wrong to > > > determine the ECC state with respect to the DQ-bus width mode. > > Sorry, but this part doesn't miss anything. Gramatically: "The IP-core reference manual says in [1,2] that the ECC support can't be enabled during the IP-core synthesizes for the DRAM data bus widths other than 16,..." "synthesizes" looks wrong. It either needs to be "... be enabled *while* the IP-core synthesizes for the DRAM..." which still doesn't make too much sense. Or "... be enabled during the IP-core *synthesis* for the DRAM..." I don't know what you mean with that "synthesizes" thing. > First of all, MSTR.data_bus_width field can have only one of the next > three values: 0x1, 0x2 and 0x3. All of them are handled in > zynqmp_get_dtype(). So in the current (incorrect) implementation it > will never return DEV_UNKNOWN. > > Secondly, dimm->dtype isn't utilized for something significant in the > EDAC subsystem, but is just exposed to the user-space via the dev_type > sysfs node. See above. > So based on that my bet is that since the incorrect code didn't affect > the main driver functionality and since the dimm->dtype is just > exposed to user-space, the bug has been living just fine unnoticed up > until I started digging into the original DW uMCTL2 HW-manuals, > started studying the driver code, and decided to convert the driver to > supporting generic version of the DW uMCTL2 controller (not only the > Xilinx version of it). That's what this series and the next two ones > are about - about converting the driver to supporting truly generic DW > uMCTL controllers. I absolutely don't have a problem with that - good idea! However, we don't break machines and don't introduce regressions. > > Can those be freely accessed? > > > > If not, you should say so. > > No, they can't be. Then you don't need to mention them. > > > > > > Fixes: b500b4a029d5 ("EDAC, synopsys: Add ECC support for ZynqMP DDR controller") > > > > So this commit is in 4.20. > > > > Does that mean that this fix needs to get backported to all stable > > kernels? > > It's up to the stable maintainers to decide. Haha, you're funny. How can the stable maintainers know whether each patch that has Fixes: tags is stable material? Nope, that's up to the maintainer to decide. > I've tested it on the devices with DW uMCTL 2.51a + DDR3 memory and DW > uMCTL 3.10a + DDR4 memory. I am sure this will work for Xilinx ZynqMP > too, especially seeing we've already got the Shubhrajyoti Datta Rb > tag: Yes, I've asked him to review that driver because this is not something I have or use and so on...
On Mon, Jun 10, 2024 at 10:00:37AM +0200, Borislav Petkov wrote: > On Wed, Jun 05, 2024 at 01:11:27AM +0300, Serge Semin wrote: > > As I said because dev_type is the memory DRAM chips type (individual > > DRAM chip data bus width), and not the entire DQ-bus width or its > > currently active part. Even from that perspective the function name > > and the subsequent return value utilization is incorrect. > > Well, maybe the author misunderstood it but the result of this goes to > sysfs: > > dimm->dtype = p_data->get_dtype(priv->baseaddr); > > which is in Documentation/ABI/testing/sysfs-devices-edac: > > What: /sys/devices/system/edac/mc/mc*/(dimm|rank)*/dimm_dev_type > Date: April 2012 > Contact: Mauro Carvalho Chehab <mchehab+samsung@kernel.org> > linux-edac@vger.kernel.org > Description: This attribute file will display what type of DRAM device is > being utilized on this DIMM (x1, x2, x4, x8, ...). > > So you'd need to fix the comment above zynqmp_get_dtype() or I can do so > too while applying. Right. I missed the comment indeed. Thanks for spotting that. If it won't be that much of a burden please fix it on merging the patch in. If I need to release v7 with this patch included, then I'll fix the comment myself. > > > First of all, not that much of the kinds. > > What does that mean: "not that much of the kinds"? The answer was following that phrase. Just two devices: ZynqMP DDRC and a DW uMCTL v3.80a-based DDRC available on the Dinh Nguyen's device. Seeing there were no much changes provided to the driver to support that controller, the controller must have been compatible with the Xilinx ZynqMP DDRC in the vast majority of the DW uMCTL2 DDRC parameters/features. > > > Just Xilinx ZynqMP DDRC (based on the DW uMCTL 2.40a IP-core) and some > > version of DW uMCTL 3.80a being possessed by Dinh Nguyen and, by > > a lucky coincident, turned to be mainly compatibly with the Xilinx > > ZynqMP DDR controller. > > Then Dinh better holler here what the story is. > > > > > 32 or 64. At the same time the bus width mode (MSTR.data_bus_width) > > > > doesn't change the ECC feature availability. Thus it was wrong to > > > > determine the ECC state with respect to the DQ-bus width mode. > > > > Sorry, but this part doesn't miss anything. > > Gramatically: > > "The IP-core reference manual says in [1,2] that the ECC support can't > be enabled during the IP-core synthesizes for the DRAM data bus widths > other than 16,..." > > "synthesizes" looks wrong. > > It either needs to be > > "... be enabled *while* the IP-core synthesizes for the DRAM..." which > still doesn't make too much sense. > > Or > > "... be enabled during the IP-core *synthesis* for the DRAM..." > > I don't know what you mean with that "synthesizes" thing. But you know what it means if "synthesis" would have been utilized, right? If no, I'll explain. If yes, then you're right. My mistake. I confused two letters. I'll fix it in v7 should the patch need to be included there. > > > First of all, MSTR.data_bus_width field can have only one of the next > > three values: 0x1, 0x2 and 0x3. All of them are handled in > > zynqmp_get_dtype(). So in the current (incorrect) implementation it > > will never return DEV_UNKNOWN. > > > > Secondly, dimm->dtype isn't utilized for something significant in the > > EDAC subsystem, but is just exposed to the user-space via the dev_type > > sysfs node. > > See above. > > > So based on that my bet is that since the incorrect code didn't affect > > the main driver functionality and since the dimm->dtype is just > > exposed to user-space, the bug has been living just fine unnoticed up > > until I started digging into the original DW uMCTL2 HW-manuals, > > started studying the driver code, and decided to convert the driver to > > supporting generic version of the DW uMCTL2 controller (not only the > > Xilinx version of it). That's what this series and the next two ones > > are about - about converting the driver to supporting truly generic DW > > uMCTL controllers. > > I absolutely don't have a problem with that - good idea! > > However, we don't break machines and don't introduce regressions. Who would have argued.) > > > > Can those be freely accessed? > > > > > > If not, you should say so. > > > > No, they can't be. > > Then you don't need to mention them. Well, I see it otherwise. If you posses the databook then by using the references you can find the info there straight away with no need in struggling through the _1.5K_ pages file. If you don't have one, then you can just skip that part of the log. So I'd rather leave the refs be in the log. > > > > > > > > > > Fixes: b500b4a029d5 ("EDAC, synopsys: Add ECC support for ZynqMP DDR controller") > > > > > > So this commit is in 4.20. > > > > > > Does that mean that this fix needs to get backported to all stable > > > kernels? > > > > It's up to the stable maintainers to decide. > > Haha, you're funny. How can the stable maintainers know whether each > patch that has Fixes: tags is stable material? > > Nope, that's up to the maintainer to decide. ... and the review committee, and the linux-kernel list members may participate in the discussion too. But that's not the point here, right? Anyway if you wished to know my opinion, then really I don't have a strong one in this patch regard. From one side the patch does fix an "oh, that's not good" issue. That's why I has added the Fixes tag. On the other hand the problem has been here unnoticed for years and nobody cared. The only parts the incorrect method implementation has affected was a wrong value returned to the user-space via the sysfs-node, and the first part of the ECC-availability test procedure which has turned to be redundant anyway since the zynqmp_get_dtype() method never returns DEV_UNKNOWN. So my conclusion is the same. It's up to the maintainers to decide. > > > I've tested it on the devices with DW uMCTL 2.51a + DDR3 memory and DW > > uMCTL 3.10a + DDR4 memory. I am sure this will work for Xilinx ZynqMP > > too, especially seeing we've already got the Shubhrajyoti Datta Rb > > tag: > > Yes, I've asked him to review that driver because this is not something > I have or use and so on... As you can see, I do and of two IP-core major versions (and plenty of the DW uMCTL2 IP-core databooks). So should you need some help with testing the bits coming for the Synopsys DW uMCTL2 EDAC driver, just send a ping to me. I'll test them out. -Serge(y) > > -- > Regards/Gruss, > Boris. > > https://people.kernel.org/tglx/notes-about-netiquette
On Tue, Jun 11, 2024 at 07:57:26PM +0300, Serge Semin wrote: > Well, I see it otherwise. If you posses the databook then by using the > references you can find the info there straight away with no need in > struggling through the _1.5K_ pages file. If you don't have one, then > you can just skip that part of the log. > > So I'd rather leave the refs be in the log. Sorry, we don't do elitist logs: only for the people who have access to some specs behind a paywall or whatnot. If those are not available freely, then you should paraphrase the relevant information into the commit message so that it explains the problem fully and people don't need to have access to those databooks and then state that the docs you reference to not publicly available. And you're doing former so I can fixup latter. > > Nope, that's up to the maintainer to decide. > > ... and the review committee, There's a review committee? I must've missed the PSA. :-P > and the linux-kernel list members may participate in the discussion too. But > that's not the point here, right? No, that's exactly the point - the maintainer and the submitter decide that - not stable people or your "committee". If someone gives a good reason why a patch should go to stable, sure, but also the maintainer's decision in the end. You know why? Because in reality the maintainer gets to mop it up after everybody and fix shit. > It's up to the maintainers to decide. Yes, what I said. As to the reason whether a patch qualifies for stable, read this here: Documentation/process/stable-kernel-rules.rst > As you can see, I do and of two IP-core major versions (and plenty of > the DW uMCTL2 IP-core databooks). So should you need some help with > testing the bits coming for the Synopsys DW uMCTL2 EDAC driver, just > send a ping to me. I'll test them out.a You can also add yourself as a reviewer for that driver and get CCed on code submissions and test them. I can surely use the help. Thx.
diff --git a/drivers/edac/synopsys_edac.c b/drivers/edac/synopsys_edac.c index 0168b05e3ca1..455d2fcfd8c1 100644 --- a/drivers/edac/synopsys_edac.c +++ b/drivers/edac/synopsys_edac.c @@ -674,26 +674,25 @@ static enum dev_type zynq_get_dtype(const void __iomem *base) */ static enum dev_type zynqmp_get_dtype(const void __iomem *base) { - enum dev_type dt; - u32 width; - - width = readl(base + CTRL_OFST); - width = (width & ECC_CTRL_BUSWIDTH_MASK) >> ECC_CTRL_BUSWIDTH_SHIFT; - switch (width) { - case DDRCTL_EWDTH_16: - dt = DEV_X2; - break; - case DDRCTL_EWDTH_32: - dt = DEV_X4; - break; - case DDRCTL_EWDTH_64: - dt = DEV_X8; - break; - default: - dt = DEV_UNKNOWN; + u32 regval; + + regval = readl(base + CTRL_OFST); + if (!(regval & MEM_TYPE_DDR4)) + return DEV_UNKNOWN; + + regval = (regval & DDRC_MSTR_CFG_MASK) >> DDRC_MSTR_CFG_SHIFT; + switch (regval) { + case DDRC_MSTR_CFG_X4_MASK: + return DEV_X4; + case DDRC_MSTR_CFG_X8_MASK: + return DEV_X8; + case DDRC_MSTR_CFG_X16_MASK: + return DEV_X16; + case DDRC_MSTR_CFG_X32_MASK: + return DEV_X32; } - return dt; + return DEV_UNKNOWN; } /** @@ -730,19 +729,11 @@ static bool zynq_get_ecc_state(void __iomem *base) */ static bool zynqmp_get_ecc_state(void __iomem *base) { - enum dev_type dt; - u32 ecctype; + u32 regval; - dt = zynqmp_get_dtype(base); - if (dt == DEV_UNKNOWN) - return false; + regval = readl(base + ECC_CFG0_OFST) & SCRUB_MODE_MASK; - ecctype = readl(base + ECC_CFG0_OFST) & SCRUB_MODE_MASK; - if ((ecctype == SCRUB_MODE_SECDED) && - ((dt == DEV_X2) || (dt == DEV_X4) || (dt == DEV_X8))) - return true; - - return false; + return (regval == SCRUB_MODE_SECDED); } /**
First of all the enum dev_type constants describe the memory DRAM chips used at the stick, not the entire DQ-bus width (see the enumeration kdoc for details). So what is returned from the zynqmp_get_dtype() function and then specified to the dimm_info->dtype field is definitely incorrect. Secondly the DRAM chips type has nothing to do with the data bus width specified in the MSTR.data_bus_width CSR field. That CSR field just determines the part of the whole DQ-bus currently used to access the data from the all DRAM memory chips. So it doesn't indicate the individual chips type. Thirdly the DRAM chips type can be determined only in case of the DDR4 protocol by means of the MSTR.device_config field state (it is supposed to be set by the system firmware). Finally the DW uMCTL2 DDRC ECC capability doesn't depend on the memory chips type. Moreover it doesn't depend on the utilized data bus width in runtime either. The IP-core reference manual says in [1,2] that the ECC support can't be enabled during the IP-core synthesizes for the DRAM data bus widths other than 16, 32 or 64. At the same time the bus width mode (MSTR.data_bus_width) doesn't change the ECC feature availability. Thus it was wrong to determine the ECC state with respect to the DQ-bus width mode. Fix all of the mistakes described above in the zynqmp_get_dtype() and zynqmp_get_ecc_state() methods: specify actual DRAM chips data width only for the DDR4 protocol and return that it's UNKNOWN in the rest of the cases; determine ECC availability by the ECCCFG0.ecc_mode field state only (that field can't be modified anyway if the IP-core was synthesized with no ECC support). [1] DesignWare® Cores Enhanced Universal DDR Memory Controller (uMCTL2) Databook, Version 3.91a, October 2020, p. 421. [2] DesignWare® Cores Enhanced Universal DDR Memory Controller (uMCTL2) Databook, Version 3.91a, October 2020, p. 633. Fixes: b500b4a029d5 ("EDAC, synopsys: Add ECC support for ZynqMP DDR controller") Signed-off-by: Serge Semin <fancer.lancer@gmail.com> --- Changelog v2: - Include "linux/bitfield.h" header file to get the FIELD_GET macro definition. (@tbot) --- drivers/edac/synopsys_edac.c | 49 +++++++++++++++--------------------- 1 file changed, 20 insertions(+), 29 deletions(-)