[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <b1454c93-375c-f4e3-0da7-291bfcc53897@canonical.com>
Date: Mon, 4 Oct 2021 10:53:55 +0200
From: Krzysztof Kozlowski <krzysztof.kozlowski@...onical.com>
To: Dmitry Osipenko <digetx@...il.com>,
Thierry Reding <thierry.reding@...il.com>,
Jonathan Hunter <jonathanh@...dia.com>,
Rob Herring <robh+dt@...nel.org>
Cc: devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-tegra@...r.kernel.org
Subject: Re: [PATCH v3 3/4] memory: Add LPDDR2 configuration helpers
On 03/10/2021 03:32, Dmitry Osipenko wrote:
> Add helpers for reading and parsing standard LPDDR2 properties.
>
> Signed-off-by: Dmitry Osipenko <digetx@...il.com>
> ---
> drivers/memory/jedec_ddr.h | 21 +++++++++++++++++
> drivers/memory/jedec_ddr_data.c | 42 +++++++++++++++++++++++++++++++++
> drivers/memory/of_memory.c | 34 ++++++++++++++++++++++++++
> drivers/memory/of_memory.h | 9 +++++++
> 4 files changed, 106 insertions(+)
>
> diff --git a/drivers/memory/jedec_ddr.h b/drivers/memory/jedec_ddr.h
> index e59ccbd982d0..14cef272559e 100644
> --- a/drivers/memory/jedec_ddr.h
> +++ b/drivers/memory/jedec_ddr.h
> @@ -230,4 +230,25 @@ struct lpddr3_min_tck {
> u32 tMRD;
> };
>
> +union lpddr2_basic_config4 {
> + u32 value;
> +
> + struct {
> + unsigned int arch_type : 2;
> + unsigned int density : 4;
> + unsigned int io_width : 2;
> + } __packed;
> +};
> +
> +struct lpddr2_configuration {
> + int arch_type;
> + int density;
> + int io_width;
> + int manufacturer_id;
> + int revision_id1;
> + int revision_id2;
> +};
> +
> +const char *lpddr2_jedec_manufacturer(unsigned int manufacturer_id);
> +
> #endif /* __JEDEC_DDR_H */
> diff --git a/drivers/memory/jedec_ddr_data.c b/drivers/memory/jedec_ddr_data.c
> index ed601d813175..1f214716ac45 100644
> --- a/drivers/memory/jedec_ddr_data.c
> +++ b/drivers/memory/jedec_ddr_data.c
> @@ -7,6 +7,7 @@
> * Aneesh V <aneesh@...com>
> */
>
> +#include <dt-bindings/memory/lpddr2.h>
> #include <linux/export.h>
>
> #include "jedec_ddr.h"
> @@ -131,3 +132,44 @@ const struct lpddr2_min_tck lpddr2_jedec_min_tck = {
> .tFAW = 8
> };
> EXPORT_SYMBOL_GPL(lpddr2_jedec_min_tck);
> +
> +const char *lpddr2_jedec_manufacturer(unsigned int manufacturer_id)
> +{
> + switch (manufacturer_id) {
> + case LPDDR2_MANID_SAMSUNG:
> + return "Samsung";
> + case LPDDR2_MANID_QIMONDA:
> + return "Qimonda";
> + case LPDDR2_MANID_ELPIDA:
> + return "Elpida";
> + case LPDDR2_MANID_ETRON:
> + return "Etron";
> + case LPDDR2_MANID_NANYA:
> + return "Nanya";
> + case LPDDR2_MANID_HYNIX:
> + return "Hynix";
> + case LPDDR2_MANID_MOSEL:
> + return "Mosel";
> + case LPDDR2_MANID_WINBOND:
> + return "Winbond";
> + case LPDDR2_MANID_ESMT:
> + return "ESMT";
> + case LPDDR2_MANID_SPANSION:
> + return "Spansion";
> + case LPDDR2_MANID_SST:
> + return "SST";
> + case LPDDR2_MANID_ZMOS:
> + return "ZMOS";
> + case LPDDR2_MANID_INTEL:
> + return "Intel";
> + case LPDDR2_MANID_NUMONYX:
> + return "Numonyx";
> + case LPDDR2_MANID_MICRON:
> + return "Micron";
> + default:
> + break;
> + }
> +
> + return "invalid";
> +}
> +EXPORT_SYMBOL_GPL(lpddr2_jedec_manufacturer);
> diff --git a/drivers/memory/of_memory.c b/drivers/memory/of_memory.c
> index d9f5437d3bce..8aa777f2a090 100644
> --- a/drivers/memory/of_memory.c
> +++ b/drivers/memory/of_memory.c
> @@ -298,3 +298,37 @@ const struct lpddr3_timings
> return NULL;
> }
> EXPORT_SYMBOL(of_lpddr3_get_ddr_timings);
> +
> +/**
> + * of_lpddr2_get_config() - extracts the lpddr2 chip configuration.
> + * @np: Pointer to device tree node containing configuration
> + * @conf: Configuration updated by this function
> + *
> + * Populates lpddr2_configuration structure by extracting data from device
> + * tree node. Returns 0 on success or error code on failure. If property
> + * is missing in device-tree, then the corresponding @conf value is set to
> + * -ENOENT.
> + */
> +int of_lpddr2_get_config(struct device_node *np,
> + struct lpddr2_configuration *conf)
> +{
Interface should be rather like of_get_ddr_timings() - allocate memory
for structure and return it. It's less error-prone.
> + int err, ret = -ENOENT;
> +
> +#define OF_LPDDR2_READ_U32(prop, dtprop) \
> + err = of_property_read_u32(np, dtprop, &conf->prop); \
> + if (err) \
> + conf->prop = -ENOENT; \
> + else \
> + ret = 0
> +
> + /* at least one property should be parsed */
> + OF_LPDDR2_READ_U32(manufacturer_id, "jedec,lpddr2-manufacturer-id");
> + OF_LPDDR2_READ_U32(revision_id1, "jedec,lpddr2-revision-id1");
> + OF_LPDDR2_READ_U32(revision_id2, "jedec,lpddr2-revision-id2");
> + OF_LPDDR2_READ_U32(io_width, "jedec,lpddr2-io-width-bits");
> + OF_LPDDR2_READ_U32(density, "jedec,lpddr2-density-mbits");
> + OF_LPDDR2_READ_U32(arch_type, "jedec,lpddr2-type");
density and io-width are required properties in existing bindings, so
return code should not be overridden.
> +
> + return ret;
> +}
> +EXPORT_SYMBOL(of_lpddr2_get_config);
> diff --git a/drivers/memory/of_memory.h b/drivers/memory/of_memory.h
> index 4a99b232ab0a..95eccc251b04 100644
> --- a/drivers/memory/of_memory.h
> +++ b/drivers/memory/of_memory.h
> @@ -20,6 +20,9 @@ const struct lpddr3_min_tck *of_lpddr3_get_min_tck(struct device_node *np,
> const struct lpddr3_timings *
> of_lpddr3_get_ddr_timings(struct device_node *np_ddr,
> struct device *dev, u32 device_type, u32 *nr_frequencies);
> +
> +int of_lpddr2_get_config(struct device_node *np,
> + struct lpddr2_configuration *conf);
> #else
> static inline const struct lpddr2_min_tck
> *of_get_min_tck(struct device_node *np, struct device *dev)
> @@ -46,6 +49,12 @@ static inline const struct lpddr3_timings
> {
> return NULL;
> }
> +
> +static int of_lpddr2_get_config(struct device_node *np,
> + struct lpddr2_configuration *conf)
> +{
> + return -ENOENT;
> +}
> #endif /* CONFIG_OF && CONFIG_DDR */
>
> #endif /* __LINUX_MEMORY_OF_REG_ */
>
Best regards,
Krzysztof
Powered by blists - more mailing lists