From: Debarati Biswas <[email protected]>
The next generation (revision 1) of the DFL EMIF feature device requires
support for more than 4 memory banks. It does not support the selective
clearing of memory banks. A capability register replaces the previous
control register, and contains a bitmask to indicate the presence of each
memory bank. This bitmask aligns with the previous control register
bitmask that served the same purpose. The control and capability
registers are treated like a C Union structure in order to support both
the new and old revisions of the EMIF device.
Signed-off-by: Debarati Biswas <[email protected]>
Signed-off-by: Russ Weight <[email protected]>
Signed-off-by: Tianfei Zhang <[email protected]>
---
v2: no code change, just rebased to v5.19-rc6.
---
drivers/memory/dfl-emif.c | 62 +++++++++++++++++++++++++++++++++++----
1 file changed, 57 insertions(+), 5 deletions(-)
diff --git a/drivers/memory/dfl-emif.c b/drivers/memory/dfl-emif.c
index 3f719816771d..da06cd30a016 100644
--- a/drivers/memory/dfl-emif.c
+++ b/drivers/memory/dfl-emif.c
@@ -24,11 +24,24 @@
#define EMIF_STAT_CLEAR_BUSY_SFT 16
#define EMIF_CTRL 0x10
#define EMIF_CTRL_CLEAR_EN_SFT 0
-#define EMIF_CTRL_CLEAR_EN_MSK GENMASK_ULL(3, 0)
+#define EMIF_CTRL_CLEAR_EN_MSK GENMASK_ULL(7, 0)
#define EMIF_POLL_INVL 10000 /* us */
#define EMIF_POLL_TIMEOUT 5000000 /* us */
+/*
+ * The Capability Register replaces the Control Register (at the same
+ * offset) for EMIF feature revisions > 0. The bitmask that indicates
+ * the presence of memory channels exists in both the Capability Register
+ * and Control Register definitions. These can be thought of as a C union.
+ * The Capability Register definitions are used to check for the existence
+ * of a memory channel, and the Control Register definitions are used for
+ * managing the memory-clear functionality in revision 0.
+ */
+#define EMIF_CAPABILITY_BASE 0x10
+#define EMIF_CAPABILITY_CHN_MSK_V0 GENMASK_ULL(3, 0)
+#define EMIF_CAPABILITY_CHN_MSK GENMASK_ULL(7, 0)
+
struct dfl_emif {
struct device *dev;
void __iomem *base;
@@ -106,16 +119,30 @@ emif_state_attr(init_done, EMIF_STAT_INIT_DONE_SFT, 0);
emif_state_attr(init_done, EMIF_STAT_INIT_DONE_SFT, 1);
emif_state_attr(init_done, EMIF_STAT_INIT_DONE_SFT, 2);
emif_state_attr(init_done, EMIF_STAT_INIT_DONE_SFT, 3);
+emif_state_attr(init_done, EMIF_STAT_INIT_DONE_SFT, 4);
+emif_state_attr(init_done, EMIF_STAT_INIT_DONE_SFT, 5);
+emif_state_attr(init_done, EMIF_STAT_INIT_DONE_SFT, 6);
+emif_state_attr(init_done, EMIF_STAT_INIT_DONE_SFT, 7);
emif_state_attr(cal_fail, EMIF_STAT_CALC_FAIL_SFT, 0);
emif_state_attr(cal_fail, EMIF_STAT_CALC_FAIL_SFT, 1);
emif_state_attr(cal_fail, EMIF_STAT_CALC_FAIL_SFT, 2);
emif_state_attr(cal_fail, EMIF_STAT_CALC_FAIL_SFT, 3);
+emif_state_attr(cal_fail, EMIF_STAT_CALC_FAIL_SFT, 4);
+emif_state_attr(cal_fail, EMIF_STAT_CALC_FAIL_SFT, 5);
+emif_state_attr(cal_fail, EMIF_STAT_CALC_FAIL_SFT, 6);
+emif_state_attr(cal_fail, EMIF_STAT_CALC_FAIL_SFT, 7);
+
emif_clear_attr(0);
emif_clear_attr(1);
emif_clear_attr(2);
emif_clear_attr(3);
+emif_clear_attr(4);
+emif_clear_attr(5);
+emif_clear_attr(6);
+emif_clear_attr(7);
+
static struct attribute *dfl_emif_attrs[] = {
&emif_attr_inf0_init_done.attr.attr,
@@ -134,6 +161,22 @@ static struct attribute *dfl_emif_attrs[] = {
&emif_attr_inf3_cal_fail.attr.attr,
&emif_attr_inf3_clear.attr.attr,
+ &emif_attr_inf4_init_done.attr.attr,
+ &emif_attr_inf4_cal_fail.attr.attr,
+ &emif_attr_inf4_clear.attr.attr,
+
+ &emif_attr_inf5_init_done.attr.attr,
+ &emif_attr_inf5_cal_fail.attr.attr,
+ &emif_attr_inf5_clear.attr.attr,
+
+ &emif_attr_inf6_init_done.attr.attr,
+ &emif_attr_inf6_cal_fail.attr.attr,
+ &emif_attr_inf6_clear.attr.attr,
+
+ &emif_attr_inf7_init_done.attr.attr,
+ &emif_attr_inf7_cal_fail.attr.attr,
+ &emif_attr_inf7_clear.attr.attr,
+
NULL,
};
@@ -143,15 +186,24 @@ static umode_t dfl_emif_visible(struct kobject *kobj,
struct dfl_emif *de = dev_get_drvdata(kobj_to_dev(kobj));
struct emif_attr *eattr = container_of(attr, struct emif_attr,
attr.attr);
+ struct dfl_device *ddev = to_dfl_dev(de->dev);
u64 val;
/*
- * This device supports upto 4 memory interfaces, but not all
+ * This device supports up to 8 memory interfaces, but not all
* interfaces are used on different platforms. The read out value of
- * CLEAN_EN field (which is a bitmap) could tell how many interfaces
- * are available.
+ * CAPABILITY_CHN_MSK field (which is a bitmap) indicates which
+ * interfaces are available.
*/
- val = FIELD_GET(EMIF_CTRL_CLEAR_EN_MSK, readq(de->base + EMIF_CTRL));
+ if (ddev->revision > 0 && strstr(attr->name, "_clear"))
+ return 0;
+
+ if (ddev->revision == 0)
+ val = FIELD_GET(EMIF_CAPABILITY_CHN_MSK_V0,
+ readq(de->base + EMIF_CAPABILITY_BASE));
+ else
+ val = FIELD_GET(EMIF_CAPABILITY_CHN_MSK,
+ readq(de->base + EMIF_CAPABILITY_BASE));
return (val & BIT_ULL(eattr->index)) ? attr->mode : 0;
}
--
2.26.2
On Wed, 13 Jul 2022, Tianfei Zhang wrote:
> From: Debarati Biswas <[email protected]>
>
> The next generation (revision 1) of the DFL EMIF feature device requires
> support for more than 4 memory banks. It does not support the selective
> clearing of memory banks. A capability register replaces the previous
> control register, and contains a bitmask to indicate the presence of each
> memory bank. This bitmask aligns with the previous control register
> bitmask that served the same purpose. The control and capability
> registers are treated like a C Union structure in order to support both
> the new and old revisions of the EMIF device.
>
> Signed-off-by: Debarati Biswas <[email protected]>
> Signed-off-by: Russ Weight <[email protected]>
> Signed-off-by: Tianfei Zhang <[email protected]>
Reviewed-by: Matthew Gerlach <[email protected]>
> ---
> v2: no code change, just rebased to v5.19-rc6.
> ---
> drivers/memory/dfl-emif.c | 62 +++++++++++++++++++++++++++++++++++----
> 1 file changed, 57 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/memory/dfl-emif.c b/drivers/memory/dfl-emif.c
> index 3f719816771d..da06cd30a016 100644
> --- a/drivers/memory/dfl-emif.c
> +++ b/drivers/memory/dfl-emif.c
> @@ -24,11 +24,24 @@
> #define EMIF_STAT_CLEAR_BUSY_SFT 16
> #define EMIF_CTRL 0x10
> #define EMIF_CTRL_CLEAR_EN_SFT 0
> -#define EMIF_CTRL_CLEAR_EN_MSK GENMASK_ULL(3, 0)
> +#define EMIF_CTRL_CLEAR_EN_MSK GENMASK_ULL(7, 0)
>
> #define EMIF_POLL_INVL 10000 /* us */
> #define EMIF_POLL_TIMEOUT 5000000 /* us */
>
> +/*
> + * The Capability Register replaces the Control Register (at the same
> + * offset) for EMIF feature revisions > 0. The bitmask that indicates
> + * the presence of memory channels exists in both the Capability Register
> + * and Control Register definitions. These can be thought of as a C union.
> + * The Capability Register definitions are used to check for the existence
> + * of a memory channel, and the Control Register definitions are used for
> + * managing the memory-clear functionality in revision 0.
> + */
> +#define EMIF_CAPABILITY_BASE 0x10
> +#define EMIF_CAPABILITY_CHN_MSK_V0 GENMASK_ULL(3, 0)
> +#define EMIF_CAPABILITY_CHN_MSK GENMASK_ULL(7, 0)
> +
> struct dfl_emif {
> struct device *dev;
> void __iomem *base;
> @@ -106,16 +119,30 @@ emif_state_attr(init_done, EMIF_STAT_INIT_DONE_SFT, 0);
> emif_state_attr(init_done, EMIF_STAT_INIT_DONE_SFT, 1);
> emif_state_attr(init_done, EMIF_STAT_INIT_DONE_SFT, 2);
> emif_state_attr(init_done, EMIF_STAT_INIT_DONE_SFT, 3);
> +emif_state_attr(init_done, EMIF_STAT_INIT_DONE_SFT, 4);
> +emif_state_attr(init_done, EMIF_STAT_INIT_DONE_SFT, 5);
> +emif_state_attr(init_done, EMIF_STAT_INIT_DONE_SFT, 6);
> +emif_state_attr(init_done, EMIF_STAT_INIT_DONE_SFT, 7);
>
> emif_state_attr(cal_fail, EMIF_STAT_CALC_FAIL_SFT, 0);
> emif_state_attr(cal_fail, EMIF_STAT_CALC_FAIL_SFT, 1);
> emif_state_attr(cal_fail, EMIF_STAT_CALC_FAIL_SFT, 2);
> emif_state_attr(cal_fail, EMIF_STAT_CALC_FAIL_SFT, 3);
> +emif_state_attr(cal_fail, EMIF_STAT_CALC_FAIL_SFT, 4);
> +emif_state_attr(cal_fail, EMIF_STAT_CALC_FAIL_SFT, 5);
> +emif_state_attr(cal_fail, EMIF_STAT_CALC_FAIL_SFT, 6);
> +emif_state_attr(cal_fail, EMIF_STAT_CALC_FAIL_SFT, 7);
> +
>
> emif_clear_attr(0);
> emif_clear_attr(1);
> emif_clear_attr(2);
> emif_clear_attr(3);
> +emif_clear_attr(4);
> +emif_clear_attr(5);
> +emif_clear_attr(6);
> +emif_clear_attr(7);
> +
>
> static struct attribute *dfl_emif_attrs[] = {
> &emif_attr_inf0_init_done.attr.attr,
> @@ -134,6 +161,22 @@ static struct attribute *dfl_emif_attrs[] = {
> &emif_attr_inf3_cal_fail.attr.attr,
> &emif_attr_inf3_clear.attr.attr,
>
> + &emif_attr_inf4_init_done.attr.attr,
> + &emif_attr_inf4_cal_fail.attr.attr,
> + &emif_attr_inf4_clear.attr.attr,
> +
> + &emif_attr_inf5_init_done.attr.attr,
> + &emif_attr_inf5_cal_fail.attr.attr,
> + &emif_attr_inf5_clear.attr.attr,
> +
> + &emif_attr_inf6_init_done.attr.attr,
> + &emif_attr_inf6_cal_fail.attr.attr,
> + &emif_attr_inf6_clear.attr.attr,
> +
> + &emif_attr_inf7_init_done.attr.attr,
> + &emif_attr_inf7_cal_fail.attr.attr,
> + &emif_attr_inf7_clear.attr.attr,
> +
> NULL,
> };
>
> @@ -143,15 +186,24 @@ static umode_t dfl_emif_visible(struct kobject *kobj,
> struct dfl_emif *de = dev_get_drvdata(kobj_to_dev(kobj));
> struct emif_attr *eattr = container_of(attr, struct emif_attr,
> attr.attr);
> + struct dfl_device *ddev = to_dfl_dev(de->dev);
> u64 val;
>
> /*
> - * This device supports upto 4 memory interfaces, but not all
> + * This device supports up to 8 memory interfaces, but not all
> * interfaces are used on different platforms. The read out value of
> - * CLEAN_EN field (which is a bitmap) could tell how many interfaces
> - * are available.
> + * CAPABILITY_CHN_MSK field (which is a bitmap) indicates which
> + * interfaces are available.
> */
> - val = FIELD_GET(EMIF_CTRL_CLEAR_EN_MSK, readq(de->base + EMIF_CTRL));
> + if (ddev->revision > 0 && strstr(attr->name, "_clear"))
> + return 0;
> +
> + if (ddev->revision == 0)
> + val = FIELD_GET(EMIF_CAPABILITY_CHN_MSK_V0,
> + readq(de->base + EMIF_CAPABILITY_BASE));
> + else
> + val = FIELD_GET(EMIF_CAPABILITY_CHN_MSK,
> + readq(de->base + EMIF_CAPABILITY_BASE));
>
> return (val & BIT_ULL(eattr->index)) ? attr->mode : 0;
> }
> --
> 2.26.2
>
>
On 13/07/2022 15:03, Tianfei Zhang wrote:
> From: Debarati Biswas <[email protected]>
>
> The next generation (revision 1) of the DFL EMIF feature device requires
> support for more than 4 memory banks. It does not support the selective
> clearing of memory banks. A capability register replaces the previous
> control register, and contains a bitmask to indicate the presence of each
> memory bank. This bitmask aligns with the previous control register
> bitmask that served the same purpose. The control and capability
> registers are treated like a C Union structure in order to support both
> the new and old revisions of the EMIF device.
>
> Signed-off-by: Debarati Biswas <[email protected]>
> Signed-off-by: Russ Weight <[email protected]>
> Signed-off-by: Tianfei Zhang <[email protected]>
> ---
Thanks for the patch and for the review.
It is too late in the cycle for me to pick it up. I will take it after
the merge window.
Best regards,
Krzysztof
On Wed, 13 Jul 2022 09:03:55 -0400, Tianfei Zhang wrote:
> From: Debarati Biswas <[email protected]>
>
> The next generation (revision 1) of the DFL EMIF feature device requires
> support for more than 4 memory banks. It does not support the selective
> clearing of memory banks. A capability register replaces the previous
> control register, and contains a bitmask to indicate the presence of each
> memory bank. This bitmask aligns with the previous control register
> bitmask that served the same purpose. The control and capability
> registers are treated like a C Union structure in order to support both
> the new and old revisions of the EMIF device.
>
> [...]
Applied, thanks!
[1/1] memory: dfl-emif: Update the dfl emif driver support revision 1
https://git.kernel.org/krzk/linux-mem-ctrl/c/d16232de82d64ce02fe6d43aa51de004755de5e5
Best regards,
--
Krzysztof Kozlowski <[email protected]>