Re: [PATCH 0/5] mtd: core: OTP nvmem provider support
Hi Rob,
Am 2021-04-20 16:08, schrieb Rob Herring:
On Fri, Apr 16, 2021 at 09:26:03PM +0200, Michael Walle wrote:
Am 2021-04-16 20:44, schrieb Rob Herring:
> On Fri, Apr 16, 2021 at 01:49:23PM +0200, Michael Walle wrote:
> > The goal is to fetch a (base) MAC address from the OTP region of a
> > SPI NOR
> > flash.
> >
> > This is the first part, where I try to add the nvmem provider
> > support to
> > the MTD core.
> >
> > I'm not sure about the device tree bindings. Consider the following
> > two
> > variants:
> >
> > (1)
> > flash@0 {
> > ..
> >
> > otp {
> > compatible = "mtd-user-otp";
>
> mtd is a linuxism. Why not just 'nvmem-cells' here or as a fallback if
> we come up with a better name?
There are two different compatibles: "mtd-user-otp" and
"mtd-factory-otp"
to differentiate what kind of OTP should be used (and both are
possible
at the same time). Thus nvmem-cells alone won't be enough. We could
drop
the "mtd-" prefix though.
Is there a benefit of having the following?
compatible = "user-otp", "nvmem-cells";
Yes. I assume 'user-otp' tells you something about the region and
'nvmem-cells' tells you that there are child nodes of nvmem data. Of
course 'user-otp' could imply 'nvmem-cells' as you did. I'm fine with
either way.
Ah, if I use both compatibles, then the
Documentation/devicetree/bindings/mtd/partitions/nvmem-cells.yaml
schema kicks in, which mandates 'compatible = "nvmem-cells";' and I
get the following errors:
CHECK Documentation/devicetree/bindings/mtd/mtd.example.dt.yaml
/home/mwalle/repos/b-linux-arm64/Documentation/devicetree/bindings/mtd/mtd.example.dt.yaml:
otp-1: compatible:0: 'nvmem-cells' was expected
From schema:
/home/mwalle/repos/linux-mw/Documentation/devicetree/bindings/mtd/partitions/nvmem-cells.yaml
/home/mwalle/repos/b-linux-arm64/Documentation/devicetree/bindings/mtd/mtd.example.dt.yaml:
otp-1: compatible: ['factory-otp', 'nvmem-cells'] is too long
From schema:
/home/mwalle/repos/linux-mw/Documentation/devicetree/bindings/mtd/partitions/nvmem-cells.yaml
/home/mwalle/repos/b-linux-arm64/Documentation/devicetree/bindings/mtd/mtd.example.dt.yaml:
otp-1: compatible: Additional items are not allowed ('nvmem-cells' was
unexpected)
From schema:
/home/mwalle/repos/linux-mw/Documentation/devicetree/bindings/mtd/partitions/nvmem-cells.yaml
Is there a way around that?
-michael
[PATCH net-next] net: enetc: automatically select IERB module
Now that enetc supports flow control we have to make sure the settings in
the IERB are correct. Therefore, we actually depend on the enetc-ierb
module. Previously it was possible that this module was disabled while the
enetc was enabled. Fix it by automatically select the enetc-ierb module.
Fixes: e7d48e5fbf30 ("net: enetc: add a mini driver for the Integrated Endpoint
Register Block")
Signed-off-by: Michael Walle
---
drivers/net/ethernet/freescale/enetc/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/freescale/enetc/Kconfig
b/drivers/net/ethernet/freescale/enetc/Kconfig
index d88f60c2bb82..cdc0ff89388a 100644
--- a/drivers/net/ethernet/freescale/enetc/Kconfig
+++ b/drivers/net/ethernet/freescale/enetc/Kconfig
@@ -2,7 +2,7 @@
config FSL_ENETC
tristate "ENETC PF driver"
depends on PCI && PCI_MSI
- depends on FSL_ENETC_IERB || FSL_ENETC_IERB=n
+ select FSL_ENETC_IERB
select FSL_ENETC_MDIO
select PHYLINK
select PCS_LYNX
--
2.20.1
[PATCH net-next v2] net: phy: at803x: fix probe error if copper page is selected
The commit c329e5afb42f ("net: phy: at803x: select correct page on
config init") selects the copper page during probe. This fails if the
copper page was already selected. In this case, the value of the copper
page (which is 1) is propagated through phy_restore_page() and is
finally returned for at803x_probe(). Fix it, by just using the
at803x_page_write() directly.
Also in case of an error, the regulator is not disabled and leads to a
WARN_ON() when the probe fails. This couldn't happen before, because
at803x_parse_dt() was the last call in at803x_probe(). It is hard to
see, that the parse_dt() actually enables the regulator. Thus move the
regulator_enable() to the probe function and undo it in case of an
error.
Fixes: c329e5afb42f ("net: phy: at803x: select correct page on config init")
Signed-off-by: Michael Walle
---
Changes since v1:
- take the bus lock
drivers/net/phy/at803x.c | 23 +--
1 file changed, 17 insertions(+), 6 deletions(-)
diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
index e0f56850edc5..32af52dd5aed 100644
--- a/drivers/net/phy/at803x.c
+++ b/drivers/net/phy/at803x.c
@@ -554,10 +554,6 @@ static int at803x_parse_dt(struct phy_device *phydev)
phydev_err(phydev, "failed to get VDDIO regulator\n");
return PTR_ERR(priv->vddio);
}
-
- ret = regulator_enable(priv->vddio);
- if (ret < 0)
- return ret;
}
return 0;
@@ -579,15 +575,30 @@ static int at803x_probe(struct phy_device *phydev)
if (ret)
return ret;
+ if (priv->vddio) {
+ ret = regulator_enable(priv->vddio);
+ if (ret < 0)
+ return ret;
+ }
+
/* Some bootloaders leave the fiber page selected.
* Switch to the copper page, as otherwise we read
* the PHY capabilities from the fiber side.
*/
if (at803x_match_phy_id(phydev, ATH8031_PHY_ID)) {
- ret = phy_select_page(phydev, AT803X_PAGE_COPPER);
- ret = phy_restore_page(phydev, AT803X_PAGE_COPPER, ret);
+ phy_lock_mdio_bus(phydev);
+ ret = at803x_write_page(phydev, AT803X_PAGE_COPPER);
+ phy_unlock_mdio_bus(phydev);
+ if (ret)
+ goto err;
}
+ return 0;
+
+err:
+ if (priv->vddio)
+ regulator_disable(priv->vddio);
+
return ret;
}
--
2.20.1
[PATCH net-next] net: phy: at803x: fix probe error if copper page is selected
The commit c329e5afb42f ("net: phy: at803x: select correct page on
config init") selects the copper page during probe. This fails if the
copper page was already selected. In this case, the value of the copper
page (which is 1) is propagated through phy_restore_page() and is
finally returned for at803x_probe(). Fix it, by just using the
at803x_page_write() directly.
Also in case of an error, the regulator is not disabled and leads to a
WARN_ON() when the probe fails. This couldn't happen before, because
at803x_parse_dt() was the last call in at803x_probe(). It is hard to
see, that the parse_dt() actually enables the regulator. Thus move the
regulator_enable() to the probe function and undo it in case of an
error.
Fixes: c329e5afb42f ("net: phy: at803x: select correct page on config init")
Signed-off-by: Michael Walle
---
drivers/net/phy/at803x.c | 21 +++--
1 file changed, 15 insertions(+), 6 deletions(-)
diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
index e0f56850edc5..5bec200f2132 100644
--- a/drivers/net/phy/at803x.c
+++ b/drivers/net/phy/at803x.c
@@ -554,10 +554,6 @@ static int at803x_parse_dt(struct phy_device *phydev)
phydev_err(phydev, "failed to get VDDIO regulator\n");
return PTR_ERR(priv->vddio);
}
-
- ret = regulator_enable(priv->vddio);
- if (ret < 0)
- return ret;
}
return 0;
@@ -579,15 +575,28 @@ static int at803x_probe(struct phy_device *phydev)
if (ret)
return ret;
+ if (priv->vddio) {
+ ret = regulator_enable(priv->vddio);
+ if (ret < 0)
+ return ret;
+ }
+
/* Some bootloaders leave the fiber page selected.
* Switch to the copper page, as otherwise we read
* the PHY capabilities from the fiber side.
*/
if (at803x_match_phy_id(phydev, ATH8031_PHY_ID)) {
- ret = phy_select_page(phydev, AT803X_PAGE_COPPER);
- ret = phy_restore_page(phydev, AT803X_PAGE_COPPER, ret);
+ ret = at803x_write_page(phydev, AT803X_PAGE_COPPER);
+ if (ret)
+ goto err;
}
+ return 0;
+
+err:
+ if (priv->vddio)
+ regulator_disable(priv->vddio);
+
return ret;
}
--
2.20.1
Re: [PATCH 0/5] mtd: core: OTP nvmem provider support
Hi Rob,
Am 2021-04-16 20:44, schrieb Rob Herring:
On Fri, Apr 16, 2021 at 01:49:23PM +0200, Michael Walle wrote:
The goal is to fetch a (base) MAC address from the OTP region of a SPI
NOR
flash.
This is the first part, where I try to add the nvmem provider support
to
the MTD core.
I'm not sure about the device tree bindings. Consider the following
two
variants:
(1)
flash@0 {
..
otp {
compatible = "mtd-user-otp";
mtd is a linuxism. Why not just 'nvmem-cells' here or as a fallback if
we come up with a better name?
There are two different compatibles: "mtd-user-otp" and
"mtd-factory-otp"
to differentiate what kind of OTP should be used (and both are possible
at the same time). Thus nvmem-cells alone won't be enough. We could drop
the "mtd-" prefix though.
Is there a benefit of having the following?
compatible = "user-otp", "nvmem-cells";
#address-cells = <1>;
#size-cells = <1>;
serial-number@0 {
reg = <0x0 0x8>;
};
};
};
(2)
flash@0 {
..
otp {
compatible = "mtd-user-otp";
#address-cells = <1>;
#size-cells = <1>;
some-useful-name {
compatible = "nvmem-cells";
serial-number@0 {
reg = <0x0 0x8>;
};
};
};
};
Both bindings use a subnode "opt[-N]". We cannot have the nvmem cells
as
children to the flash node because of the legacy partition binding.
(1) seems to be the form which is used almost everywhere in the
kernel.
That is, the nvmem cells are just children of the parent node.
(2) seem to be more natural, because there might also be other
properties
inside the otp subnode and might be more future-proof.
At the moment this patch implements (1).
I think approach (1) seems fine.
ok
-michael
[PATCH 5/5] mtd: core: add OTP nvmem provider support
Flash OTP regions can already be read via user space. Some boards have
their serial number or MAC addresses stored in the OTP regions. Add
support for them being a (read-only) nvmem provider.
The API to read the OTP data is already in place. It distinguishes
between factory and user OTP, thus there are up to two different
providers.
Signed-off-by: Michael Walle
---
Changes since RFC:
- none
drivers/mtd/mtdcore.c | 149
include/linux/mtd/mtd.h | 2 +
2 files changed, 151 insertions(+)
diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
index 0bc6871c3863..92201e3d187a 100644
--- a/drivers/mtd/mtdcore.c
+++ b/drivers/mtd/mtdcore.c
@@ -777,6 +777,147 @@ static void mtd_set_dev_defaults(struct mtd_info *mtd)
mutex_init(>master.chrdev_lock);
}
+static ssize_t mtd_otp_size(struct mtd_info *mtd, bool is_user)
+{
+ struct otp_info *info = kmalloc(PAGE_SIZE, GFP_KERNEL);
+ ssize_t size = 0;
+ unsigned int i;
+ size_t retlen;
+ int ret;
+
+ if (is_user)
+ ret = mtd_get_user_prot_info(mtd, PAGE_SIZE, , info);
+ else
+ ret = mtd_get_fact_prot_info(mtd, PAGE_SIZE, , info);
+ if (ret)
+ goto err;
+
+ for (i = 0; i < retlen / sizeof(*info); i++) {
+ size += info->length;
+ info++;
+ }
+
+ kfree(info);
+ return size;
+
+err:
+ kfree(info);
+ return ret;
+}
+
+static struct nvmem_device *mtd_otp_nvmem_register(struct mtd_info *mtd,
+ const char *name, int size,
+ nvmem_reg_read_t reg_read,
+ const char *compatible)
+{
+ struct nvmem_device *nvmem = NULL;
+ struct nvmem_config config = {};
+ struct device_node *np;
+
+ /* DT binding is optional */
+ np = of_get_compatible_child(mtd->dev.of_node, compatible);
+
+ /* OTP nvmem will be registered on the physical device */
+ config.dev = mtd->dev.parent;
+ config.name = name;
+ config.id = NVMEM_DEVID_NONE;
+ config.owner = THIS_MODULE;
+ config.type = NVMEM_TYPE_OTP;
+ config.root_only = true;
+ config.reg_read = reg_read;
+ config.size = size;
+ config.of_node = np;
+ config.priv = mtd;
+
+ nvmem = nvmem_register();
+ /* Just ignore if there is no NVMEM support in the kernel */
+ if (IS_ERR(nvmem) && PTR_ERR(nvmem) == -EOPNOTSUPP)
+ nvmem = NULL;
+
+ of_node_put(np);
+
+ return nvmem;
+}
+
+static int mtd_nvmem_user_otp_reg_read(void *priv, unsigned int offset,
+ void *val, size_t bytes)
+{
+ struct mtd_info *mtd = priv;
+ size_t retlen;
+ int ret;
+
+ ret = mtd_read_user_prot_reg(mtd, offset, bytes, , val);
+ if (ret)
+ return ret;
+
+ return retlen == bytes ? 0 : -EIO;
+}
+
+static int mtd_nvmem_fact_otp_reg_read(void *priv, unsigned int offset,
+ void *val, size_t bytes)
+{
+ struct mtd_info *mtd = priv;
+ size_t retlen;
+ int ret;
+
+ ret = mtd_read_fact_prot_reg(mtd, offset, bytes, , val);
+ if (ret)
+ return ret;
+
+ return retlen == bytes ? 0 : -EIO;
+}
+
+static int mtd_otp_nvmem_add(struct mtd_info *mtd)
+{
+ struct nvmem_device *nvmem;
+ ssize_t size;
+ int err;
+
+ if (mtd->_get_user_prot_info && mtd->_read_user_prot_reg) {
+ size = mtd_otp_size(mtd, true);
+ if (size < 0)
+ return size;
+
+ if (size > 0) {
+ nvmem = mtd_otp_nvmem_register(mtd, "user-otp", size,
+
mtd_nvmem_user_otp_reg_read,
+ "mtd-user-otp");
+ if (IS_ERR(nvmem)) {
+ dev_err(>dev, "Failed to register OTP
NVMEM device\n");
+ return PTR_ERR(nvmem);
+ }
+ mtd->otp_user_nvmem = nvmem;
+ }
+ }
+
+ if (mtd->_get_fact_prot_info && mtd->_read_fact_prot_reg) {
+ size = mtd_otp_size(mtd, false);
+ if (size < 0) {
+ err = size;
+ goto err;
+ }
+
+ if (size > 0) {
+ nvmem = mtd_otp_nvmem_register(mtd, "factory-otp", size,
+
mtd_nvmem_fact_otp_reg_read,
+ "mtd-factory-otp");
+ if (IS_ERR(nvmem)) {
+
[PATCH 2/5] dt-bindings: mtd: add YAML schema for the generic MTD bindings
Convert MTD's common.txt to mtd.yaml.
Signed-off-by: Michael Walle
Reviewed-by: Rob Herring
---
Changes since RFC:
- use real device compatibles
.../devicetree/bindings/mtd/common.txt| 16 +---
.../devicetree/bindings/mtd/mtd.yaml | 39 +++
2 files changed, 40 insertions(+), 15 deletions(-)
create mode 100644 Documentation/devicetree/bindings/mtd/mtd.yaml
diff --git a/Documentation/devicetree/bindings/mtd/common.txt
b/Documentation/devicetree/bindings/mtd/common.txt
index fc068b923d7a..ae16f9ea8606 100644
--- a/Documentation/devicetree/bindings/mtd/common.txt
+++ b/Documentation/devicetree/bindings/mtd/common.txt
@@ -1,15 +1 @@
-* Common properties of all MTD devices
-
-Optional properties:
-- label: user-defined MTD device name. Can be used to assign user
- friendly names to MTD devices (instead of the flash model or flash
- controller based name) in order to ease flash device identification
- and/or describe what they are used for.
-
-Example:
-
- flash@0 {
- label = "System-firmware";
-
- /* flash type specific properties */
- };
+This file has been moved to mtd.yaml.
diff --git a/Documentation/devicetree/bindings/mtd/mtd.yaml
b/Documentation/devicetree/bindings/mtd/mtd.yaml
new file mode 100644
index ..086b0ecd1604
--- /dev/null
+++ b/Documentation/devicetree/bindings/mtd/mtd.yaml
@@ -0,0 +1,39 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mtd/mtd.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: MTD (Memory Technology Device) Device Tree Bindings
+
+maintainers:
+ - Miquel Raynal
+ - Richard Weinberger
+
+properties:
+ $nodename:
+pattern: "^flash(@.*)?$"
+
+ label:
+description:
+ User-defined MTD device name. Can be used to assign user friendly
+ names to MTD devices (instead of the flash model or flash controller
+ based name) in order to ease flash device identification and/or
+ describe what they are used for.
+
+additionalProperties: true
+
+examples:
+ - |
+spi {
+#address-cells = <1>;
+#size-cells = <0>;
+
+flash@0 {
+reg = <0>;
+compatible = "jedec,spi-nor";
+label = "System-firmware";
+};
+};
+
+...
--
2.20.1
[PATCH 4/5] dt-bindings: mtd: spi-nor: add otp property
SPI-NOR flashes may have OTP regions and have a nvmem binding. This binding is described in mtd.yaml. Signed-off-by: Michael Walle --- Changes since RFC: - new patch Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml | 6 ++ 1 file changed, 6 insertions(+) diff --git a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml index 5e7e5349f9a1..ed590d7c6e37 100644 --- a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml +++ b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml @@ -9,6 +9,9 @@ title: SPI NOR flash ST M25Pxx (and similar) serial flash chips maintainers: - Rob Herring +allOf: + - $ref: "mtd.yaml#" + properties: compatible: oneOf: @@ -82,6 +85,9 @@ patternProperties: '^partition@': type: object + "^otp(-[0-9]+)?$": +type: object + additionalProperties: false examples: -- 2.20.1
[PATCH 3/5] dt-bindings: mtd: add OTP bindings
Flash devices can have one-time-programmable regions. Add a nvmem
binding so they can be used as a nvmem provider.
Signed-off-by: Michael Walle
---
Changes since RFC:
- added missing "$"
- dropped first example
- use real device compatibles
Please note, that this will lead to an error without patch 4/5, which
introduces that property for the jedec,spi-nor. Should I keep it
seperate or should I squash that patch into this one?
.../devicetree/bindings/mtd/mtd.yaml | 50 +++
1 file changed, 50 insertions(+)
diff --git a/Documentation/devicetree/bindings/mtd/mtd.yaml
b/Documentation/devicetree/bindings/mtd/mtd.yaml
index 086b0ecd1604..dd43fb8b4fd1 100644
--- a/Documentation/devicetree/bindings/mtd/mtd.yaml
+++ b/Documentation/devicetree/bindings/mtd/mtd.yaml
@@ -21,6 +21,25 @@ properties:
based name) in order to ease flash device identification and/or
describe what they are used for.
+patternProperties:
+ "^otp(-[0-9]+)?$":
+type: object
+$ref: ../nvmem/nvmem.yaml#
+
+description: |
+ An OTP memory region. Some flashes provide a one-time-programmable
+ memory whose content can either be programmed by a user or is already
+ pre-programmed by the factory. Some flashes might provide both.
+
+properties:
+ compatible:
+enum:
+ - mtd-user-otp
+ - mtd-factory-otp
+
+required:
+ - compatible
+
additionalProperties: true
examples:
@@ -36,4 +55,35 @@ examples:
};
};
+ - |
+spi {
+#address-cells = <1>;
+#size-cells = <0>;
+
+flash@0 {
+reg = <0>;
+compatible = "jedec,spi-nor";
+
+otp-1 {
+compatible = "mtd-factory-otp";
+#address-cells = <1>;
+#size-cells = <1>;
+
+electronic-serial-number@0 {
+reg = <0 8>;
+};
+};
+
+otp-2 {
+compatible = "mtd-user-otp";
+#address-cells = <1>;
+#size-cells = <1>;
+
+mac-address@0 {
+reg = <0 6>;
+};
+};
+};
+};
+
...
--
2.20.1
[PATCH 1/5] nvmem: core: allow specifying of_node
Until now, the of_node of the parent device is used. Some devices
provide more than just the nvmem provider. To avoid name space clashes,
add a way to allow specifying the nvmem cells in subnodes. Consider the
following example:
flash@0 {
compatible = "jedec,spi-nor";
partitions {
compatible = "fixed-partitions";
#address-cells = <1>;
#size-cells = <1>;
partition@0 {
reg = <0x00 0x01>;
};
};
otp {
compatible = "mtd-user-otp";
#address-cells = <1>;
#size-cells = <1>;
serial-number@0 {
reg = <0x0 0x8>;
};
};
};
There the nvmem provider might be the MTD partition or the OTP region of
the flash.
Add a new config->of_node parameter, which if set, will be used instead
of the parent's of_node.
Signed-off-by: Michael Walle
Acked-by: Srinivas Kandagatla
---
Changes since RFC:
- none
drivers/nvmem/core.c | 4 +++-
include/linux/nvmem-provider.h | 2 ++
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
index bca671ff4e54..62d363a399d3 100644
--- a/drivers/nvmem/core.c
+++ b/drivers/nvmem/core.c
@@ -789,7 +789,9 @@ struct nvmem_device *nvmem_register(const struct
nvmem_config *config)
nvmem->reg_write = config->reg_write;
nvmem->keepout = config->keepout;
nvmem->nkeepout = config->nkeepout;
- if (!config->no_of_node)
+ if (config->of_node)
+ nvmem->dev.of_node = config->of_node;
+ else if (!config->no_of_node)
nvmem->dev.of_node = config->dev->of_node;
switch (config->id) {
diff --git a/include/linux/nvmem-provider.h b/include/linux/nvmem-provider.h
index e162b757b6d5..471cb7b9e896 100644
--- a/include/linux/nvmem-provider.h
+++ b/include/linux/nvmem-provider.h
@@ -57,6 +57,7 @@ struct nvmem_keepout {
* @type: Type of the nvmem storage
* @read_only: Device is read-only.
* @root_only: Device is accessibly to root only.
+ * @of_node: If given, this will be used instead of the parent's of_node.
* @no_of_node:Device should not use the parent's of_node even if it's
!NULL.
* @reg_read: Callback to read data.
* @reg_write: Callback to write data.
@@ -86,6 +87,7 @@ struct nvmem_config {
enum nvmem_type type;
boolread_only;
boolroot_only;
+ struct device_node *of_node;
boolno_of_node;
nvmem_reg_read_treg_read;
nvmem_reg_write_t reg_write;
--
2.20.1
[PATCH 0/5] mtd: core: OTP nvmem provider support
The goal is to fetch a (base) MAC address from the OTP region of a SPI NOR
flash.
This is the first part, where I try to add the nvmem provider support to
the MTD core.
I'm not sure about the device tree bindings. Consider the following two
variants:
(1)
flash@0 {
..
otp {
compatible = "mtd-user-otp";
#address-cells = <1>;
#size-cells = <1>;
serial-number@0 {
reg = <0x0 0x8>;
};
};
};
(2)
flash@0 {
..
otp {
compatible = "mtd-user-otp";
#address-cells = <1>;
#size-cells = <1>;
some-useful-name {
compatible = "nvmem-cells";
serial-number@0 {
reg = <0x0 0x8>;
};
};
};
};
Both bindings use a subnode "opt[-N]". We cannot have the nvmem cells as
children to the flash node because of the legacy partition binding.
(1) seems to be the form which is used almost everywhere in the kernel.
That is, the nvmem cells are just children of the parent node.
(2) seem to be more natural, because there might also be other properties
inside the otp subnode and might be more future-proof.
At the moment this patch implements (1).
Michael Walle (5):
nvmem: core: allow specifying of_node
dt-bindings: mtd: add YAML schema for the generic MTD bindings
dt-bindings: mtd: add OTP bindings
dt-bindings: mtd: spi-nor: add otp property
mtd: core: add OTP nvmem provider support
.../devicetree/bindings/mtd/common.txt| 16 +-
.../bindings/mtd/jedec,spi-nor.yaml | 6 +
.../devicetree/bindings/mtd/mtd.yaml | 89 +++
drivers/mtd/mtdcore.c | 149 ++
drivers/nvmem/core.c | 4 +-
include/linux/mtd/mtd.h | 2 +
include/linux/nvmem-provider.h| 2 +
7 files changed, 252 insertions(+), 16 deletions(-)
create mode 100644 Documentation/devicetree/bindings/mtd/mtd.yaml
--
2.20.1
Re: [PATCH net-next v4 2/2] of: net: fix of_get_mac_addr_nvmem() for non-platform devices
Am 2021-04-16 05:24, schrieb Benjamin Herrenschmidt:
On Mon, 2021-04-12 at 19:47 +0200, Michael Walle wrote:
/**
* of_get_phy_mode - Get phy mode for given device_node
@@ -59,15 +60,39 @@ static int of_get_mac_addr(struct device_node *np,
const char *name, u8 *addr)
static int of_get_mac_addr_nvmem(struct device_node *np, u8 *addr)
{
struct platform_device *pdev = of_find_device_by_node(np);
+ struct nvmem_cell *cell;
+ const void *mac;
+ size_t len;
int ret;
- if (!pdev)
- return -ENODEV;
+ /* Try lookup by device first, there might be a
nvmem_cell_lookup
+* associated with a given device.
+*/
+ if (pdev) {
+ ret = nvmem_get_mac_address(>dev, addr);
+ put_device(>dev);
+ return ret;
+ }
+
This smells like the wrong band aid :)
Any struct device can contain an OF node pointer these days.
But not all nodes might have an associated device, see DSA for example.
And as the name suggests of_get_mac_address() operates on a node. So
if a driver calls of_get_mac_address() it should work on the node. What
is wrong IMHO, is that the ethernet drivers where the corresponding
board
has a nvmem_cell_lookup registered is calling of_get_mac_address(node).
It should rather call eth_get_mac_address(dev) in the first place.
One would need to figure out if there is an actual device (with an
assiciated of_node), then call eth_get_mac_address(dev) and if there
isn't a device call of_get_mac_address(node).
But I don't know if that is easy to figure out. Well, one could start
with just the device where nvmem_cell_lookup is used. Then we could
drop the workaround above.
This seems all backwards. I think we are dealing with bad evolution.
We need to do a lookup for the device because we get passed an of_node.
We should just get passed a device here... or rather stop calling
of_get_mac_addr() from all those drivers and instead call
eth_platform_get_mac_address() which in turns calls of_get_mac_addr().
Then the nvmem stuff gets put in eth_platform_get_mac_address().
of_get_mac_addr() becomes a low-level thingy that most drivers don't
care about.
The NVMEM thing is just another (optional) way how the MAC address
is fetched from the device tree. Thus, if the drivers have the
of_get_mac_address() call they should automatically get the NVMEM
method, too.
-michael
Re: [PATCH net-next 1/3] dt-bindings: net: add nvmem-mac-address-offset property
Am 2021-04-15 23:59, schrieb Rob Herring: On Wed, Apr 14, 2021 at 05:43:49PM +0200, Andrew Lunn wrote: On Wed, Apr 14, 2021 at 05:26:55PM +0200, Michael Walle wrote: > It is already possible to read the MAC address via a NVMEM provider. But > there are boards, esp. with many ports, which only have a base MAC > address stored. Thus we need to have a way to provide an offset per > network device. We need to see what Rob thinks of this. There was recently a patchset to support swapping the byte order of the MAC address in a NVMEM. Rob said the NVMEM provider should have the property, not the MAC driver. This does seems more ethernet specific, so maybe it should be an Ethernet property? There was also this one[1]. I'm not totally opposed, but don't want to see a never ending addition of properties to try to describe any possible transformation. Agreed, that stuff like ASCII MAC address parsing should be done elsewhere. But IMHO adding an offset is a pretty common one (as also pointed out in [1]). And it also need to be a per ethernet device property. -michael [1] https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20200920095724.8251-4-ansuels...@gmail.com/
[PATCH net-next 3/3] net: implement nvmem-mac-address-offset DT property
The MAC address fetched by an NVMEM provider might have an offset to it.
Add the support for it in nvmem_get_mac_address().
Signed-off-by: Michael Walle
---
drivers/of/of_net.c | 4
net/ethernet/eth.c | 5 +
2 files changed, 9 insertions(+)
diff --git a/drivers/of/of_net.c b/drivers/of/of_net.c
index dbac3a172a11..60c6048a823a 100644
--- a/drivers/of/of_net.c
+++ b/drivers/of/of_net.c
@@ -63,6 +63,7 @@ static int of_get_mac_addr_nvmem(struct device_node *np, u8
*addr)
struct nvmem_cell *cell;
const void *mac;
size_t len;
+ u32 offset;
int ret;
/* Try lookup by device first, there might be a nvmem_cell_lookup
@@ -92,6 +93,9 @@ static int of_get_mac_addr_nvmem(struct device_node *np, u8
*addr)
memcpy(addr, mac, ETH_ALEN);
kfree(mac);
+ if (!of_property_read_u32(np, "nvmem-mac-address-offset", ))
+ eth_addr_add(addr, offset);
+
return 0;
}
diff --git a/net/ethernet/eth.c b/net/ethernet/eth.c
index 9cce612e8976..fe5311f614fe 100644
--- a/net/ethernet/eth.c
+++ b/net/ethernet/eth.c
@@ -541,6 +541,7 @@ int nvmem_get_mac_address(struct device *dev, void *addrbuf)
{
struct nvmem_cell *cell;
const void *mac;
+ u32 offset;
size_t len;
cell = nvmem_cell_get(dev, "mac-address");
@@ -561,6 +562,10 @@ int nvmem_get_mac_address(struct device *dev, void
*addrbuf)
ether_addr_copy(addrbuf, mac);
kfree(mac);
+ if (!device_property_read_u32(dev, "nvmem-mac-address-offset",
+ ))
+ eth_addr_add(addrbuf, offset);
+
return 0;
}
EXPORT_SYMBOL(nvmem_get_mac_address);
--
2.20.1
[PATCH net-next 1/3] dt-bindings: net: add nvmem-mac-address-offset property
It is already possible to read the MAC address via a NVMEM provider. But there are boards, esp. with many ports, which only have a base MAC address stored. Thus we need to have a way to provide an offset per network device. Signed-off-by: Michael Walle --- .../devicetree/bindings/net/ethernet-controller.yaml| 6 ++ 1 file changed, 6 insertions(+) diff --git a/Documentation/devicetree/bindings/net/ethernet-controller.yaml b/Documentation/devicetree/bindings/net/ethernet-controller.yaml index e8f04687a3e0..1a8517b0e445 100644 --- a/Documentation/devicetree/bindings/net/ethernet-controller.yaml +++ b/Documentation/devicetree/bindings/net/ethernet-controller.yaml @@ -52,6 +52,12 @@ properties: nvmem-cell-names: const: mac-address + nvmem-mac-address-offset: +maxItems: 1 +description: + Specifies an offset which will be added to the MAC address when + fetched from a nvmem cell. + phy-connection-type: description: Specifies interface type between the Ethernet device and a physical -- 2.20.1
[PATCH net-next 2/3] net: add helper eth_addr_add()
Sometimes you need to add an offset to a base ethernet address. Add a
helper for that.
Signed-off-by: Michael Walle
---
include/linux/etherdevice.h | 14 ++
1 file changed, 14 insertions(+)
diff --git a/include/linux/etherdevice.h b/include/linux/etherdevice.h
index 330345b1be54..6ec62c501d3f 100644
--- a/include/linux/etherdevice.h
+++ b/include/linux/etherdevice.h
@@ -466,6 +466,20 @@ static inline void eth_addr_inc(u8 *addr)
u64_to_ether_addr(u, addr);
}
+/**
+ * eth_addr_add() - Add (or subtract) and offset to/from the given MAC address.
+ *
+ * @offset: Offset to add.
+ * @addr: Pointer to a six-byte array containing Ethernet address to increment.
+ */
+static inline void eth_addr_add(u8 *addr, long offset)
+{
+ u64 u = ether_addr_to_u64(addr);
+
+ u += offset;
+ u64_to_ether_addr(u, addr);
+}
+
/**
* is_etherdev_addr - Tell if given Ethernet address belongs to the device.
* @dev: Pointer to a device structure
--
2.20.1
[PATCH net-next 0/3] net: add support for an offset of a nvmem provided MAC address
Boards with multiple ethernet ports might store their MAC addresses not individually per port but just store one base MAC address. To get the MAC address of a specific network port we have to add an offset. This series adds a new device tree property "nvmem-mac-address-offset". Michael Walle (3): dt-bindings: net: add nvmem-mac-address-offset property net: add helper eth_addr_add() net: implement nvmem-mac-address-offset DT property .../bindings/net/ethernet-controller.yaml | 6 ++ drivers/of/of_net.c| 4 include/linux/etherdevice.h| 14 ++ net/ethernet/eth.c | 5 + 4 files changed, 29 insertions(+) -- 2.20.1
[PATCH net-next] net: enetc: fetch MAC address from device tree
Normally, the bootloader will already initialize the MAC address
registers of the ENETC and the driver will just use them or generate a
random one, if it is not initialized.
Add a new way to provide the MAC address: via device tree. Besides the
usual 'mac-address' property, there is also the possibility to fetch it
via a NVMEM provider. The sl28 board stores the MAC address in the SPI
NOR flash OTP region. Having this will allow linux to fetch the MAC
address from there without being dependent on the bootloader.
No in-tree boards have the device tree properties set, thus for these,
this is a no-op.
Signed-off-by: Michael Walle
---
.../net/ethernet/freescale/enetc/enetc_pf.c | 65 ++-
1 file changed, 49 insertions(+), 16 deletions(-)
diff --git a/drivers/net/ethernet/freescale/enetc/enetc_pf.c
b/drivers/net/ethernet/freescale/enetc/enetc_pf.c
index f61fedf462e5..30b22b71630e 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc_pf.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc_pf.c
@@ -390,23 +390,54 @@ static int enetc_pf_set_vf_spoofchk(struct net_device
*ndev, int vf, bool en)
return 0;
}
-static void enetc_port_setup_primary_mac_address(struct enetc_si *si)
+static int enetc_setup_mac_address(struct device_node *np, struct enetc_pf *pf,
+ int si)
{
- unsigned char mac_addr[MAX_ADDR_LEN];
- struct enetc_pf *pf = enetc_si_priv(si);
- struct enetc_hw *hw = >hw;
- int i;
+ struct device *dev = >si->pdev->dev;
+ struct enetc_hw *hw = >si->hw;
+ u8 mac_addr[ETH_ALEN] = { 0 };
+ int err;
- /* check MAC addresses for PF and all VFs, if any is 0 set it ro rand */
- for (i = 0; i < pf->total_vfs + 1; i++) {
- enetc_pf_get_primary_mac_addr(hw, i, mac_addr);
- if (!is_zero_ether_addr(mac_addr))
- continue;
+ /* (1) try to get the MAC address from the device tree */
+ if (np) {
+ err = of_get_mac_address(np, mac_addr);
+ if (err == -EPROBE_DEFER)
+ return err;
+ }
+
+ /* (2) bootloader supplied MAC address */
+ if (is_zero_ether_addr(mac_addr))
+ enetc_pf_get_primary_mac_addr(hw, si, mac_addr);
+
+ /* (3) choose a random one */
+ if (is_zero_ether_addr(mac_addr)) {
eth_random_addr(mac_addr);
- dev_info(>pdev->dev, "no MAC address specified for SI%d,
using %pM\n",
-i, mac_addr);
- enetc_pf_set_primary_mac_addr(hw, i, mac_addr);
+ dev_info(dev, "no MAC address specified for SI%d, using %pM\n",
+si, mac_addr);
}
+
+ enetc_pf_set_primary_mac_addr(hw, si, mac_addr);
+
+ return 0;
+}
+
+static int enetc_setup_mac_addresses(struct device_node *np,
+struct enetc_pf *pf)
+{
+ int err, i;
+
+ /* The PF might take its MAC from the device tree */
+ err = enetc_setup_mac_address(np, pf, 0);
+ if (err)
+ return err;
+
+ for (i = 0; i < pf->total_vfs; i++) {
+ err = enetc_setup_mac_address(NULL, pf, i + 1);
+ if (err)
+ return err;
+ }
+
+ return 0;
}
static void enetc_port_assign_rfs_entries(struct enetc_si *si)
@@ -562,9 +593,6 @@ static void enetc_configure_port(struct enetc_pf *pf)
/* split up RFS entries */
enetc_port_assign_rfs_entries(pf->si);
- /* fix-up primary MAC addresses, if not set already */
- enetc_port_setup_primary_mac_address(pf->si);
-
/* enforce VLAN promisc mode for all SIs */
pf->vlan_promisc_simap = ENETC_VLAN_PROMISC_MAP_ALL;
enetc_set_vlan_promisc(hw, pf->vlan_promisc_simap);
@@ -1137,6 +1165,10 @@ static int enetc_pf_probe(struct pci_dev *pdev,
pf->si = si;
pf->total_vfs = pci_sriov_get_totalvfs(pdev);
+ err = enetc_setup_mac_addresses(node, pf);
+ if (err)
+ goto err_setup_mac_addresses;
+
enetc_configure_port(pf);
enetc_get_si_caps(si);
@@ -1204,6 +1236,7 @@ static int enetc_pf_probe(struct pci_dev *pdev,
err_init_port_rss:
err_init_port_rfs:
err_device_disabled:
+err_setup_mac_addresses:
enetc_teardown_cbdr(>cbd_ring);
err_setup_cbdr:
err_map_pf_space:
--
2.20.1
Re: [PATCH net-next v2 1/2] of: net: pass the dst buffer to of_get_mac_address()
Hi Dan,
Am 2021-04-14 07:33, schrieb Dan Carpenter:
url:
https://github.com/0day-ci/linux/commits/Michael-Walle/of-net-support-non-platform-devices-in-of_get_mac_address/20210406-234030
base:
https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git
cc0626c2aaed8e475efdd85fa374b497a7192e35
config: x86_64-randconfig-m001-20210406 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
Reported-by: Dan Carpenter
smatch warnings:
drivers/net/ethernet/xilinx/xilinx_axienet_main.c:2069 axienet_probe()
warn: passing a valid pointer to 'PTR_ERR'
vim +/PTR_ERR +2069 drivers/net/ethernet/xilinx/xilinx_axienet_main.c
522856cefaf09d Robert Hancock 2019-06-06 2060 /* Check for
Ethernet core IRQ (optional) */
522856cefaf09d Robert Hancock 2019-06-06 2061 if (lp->eth_irq
<= 0)
522856cefaf09d Robert Hancock 2019-06-06 2062
dev_info(>dev, "Ethernet core IRQ not defined\n");
522856cefaf09d Robert Hancock 2019-06-06 2063
8a3b7a252dca9f Daniel Borkmann 2012-01-19 2064 /* Retrieve the
MAC address */
411b125c6ace1f Michael Walle 2021-04-06 2065 ret =
of_get_mac_address(pdev->dev.of_node, mac_addr);
411b125c6ace1f Michael Walle 2021-04-06 2066 if (!ret) {
411b125c6ace1f Michael Walle 2021-04-06 2067
axienet_set_mac_address(ndev, mac_addr);
411b125c6ace1f Michael Walle 2021-04-06 2068 } else {
d05a9ed5c3a773 Robert Hancock 2019-06-06 @2069
dev_warn(>dev, "could not find MAC address property:
%ld\n",
d05a9ed5c3a773 Robert Hancock 2019-06-06 2070
PTR_ERR(mac_addr));
^
This should print "ret".
Thanks, this was fixed (in the now merged) v4. I forgot
to add you to that huge CC list. Sorry for that.
-michael
Re: [PATCH] mtd: spi-nor: macronix: Add block protection support to mx25u6435f
Hi, Am 2021-04-14 08:53, schrieb Ikjoon Jang: On Tue, Apr 13, 2021 at 8:26 PM Michael Walle wrote: Am 2021-04-13 14:02, schrieb Ikjoon Jang: > This patch adds block protection support to Macronix mx25u6432f and > mx25u6435f. Two different chips share the same JEDEC ID while only > mx25u6423f support section protections. And two chips have slightly > different definitions of BP bits than generic (ST Micro) > implementation. What is different compared to the current implementation? Could you give an example? Two chips have different definitions on BP and T/B bits. - 35f has 4 BPs without T/B, BP3 behaves like T/B bit. See below. - 32f has 4 BPs plus T/B on CR. Ok, so this scheme looks like what we have right now, only that the TB bit is OTP and at a different place, right? # MX25U6435F BPs | BP3=0 | BP3=1 - 000 | None | 1/2 001 | 1/128 | 3/4 010 | 1/64 | 7/8 011 | 1/32 | 15/16 100 | 1/16 | 31/32 101 | 1/8 | 63/64 110 | 1/4 | 127/128 111 | 1/2 | All # MX25U6432F BPs | T/B=0 | T/B=1 - | None | None 0001 | 1/128 | 1/128 0010 | 1/64 | 1/64 0011 | 1/32 | 1/32 0100 | 1/16 | 1/16 0101 | 1/8 | 1/8 0110 | 1/4 | 1/4 0111 | 1/2 | 1/2 1xxx | All | All It seems that 35f has a unique definition on bottom protections than others. Oh my.. That looks more like an invert and the top protection is also different. That is, if you'd treat BP3 as T/B, then BP[2:0] = 111 should be "lock all", but it is rather lock half.. I just looked at the mx25u3235f back then. There it looked correct. But now it looks like the top protection scheme clips on the lower end (i.e. always starts with 1 block), where on the current supported scheme, we clip on the upper end (i.e. we start with protect all and walk backwards). Assuming there's no way to distinguish between mx25u6435f and 32f, This patch simply takes the common parts only - BP[2:0] without using T/B or BP3 at all. You could look for differences in the SFDP and then distiguish them during probe and set the corresponding flags. Where the flags would need to be implemented first. I wouldn't have a problem with saying we just support top protection for the MX25U6435F but then we'd need to make sure the BP3 is set to 0. If you want to read out the SFDP, see here: https://patchwork.ozlabs.org/project/linux-mtd/list/?series=235475 But the current swp implementation implies that "BP with all ones" is to be 'all' protection while in this approach it's 1/2, A hidden BP3 should be involved for 'all' protection. Yes, so the MX25U6432F needs to have the 4bit BP flag set and the MX25U6435F seems to be completely different. Doh.. > So this patch defines a new spi_nor_locking_ops only for macronix > until this could be merged into a generic swp implementation. TBH, I don't really like the code duplication and I'd presume that it won't ever be merged with the generic code. Agree, I hope I could make a more generalized version into swp.c. Honestly I expected that I just needed to add one line of SPI_NOR_HAS_LOCK to flash_info to support mx256432f (this was the main purpose of my patch) before I read the datasheets. :) You also assume that both the WPSEL and T/B bit are 0, which might not be true. Please note that both are write-once, thus should only be read. yep, that also should be considered, I'm thinking just not to support WPSEL=1 case for now. which is ok, but we should make sure it is not set at all. Now the question is what do we do if it is set? I'd say just don't register the locking ops and log a message. -michael
[PATCH] rtc: fsl-ftm-alarm: add MODULE_TABLE()
The module doesn't load automatically. Fix it by adding the missing
MODULE_TABLE().
Fixes: 7b0b551dbc1e ("rtc: fsl-ftm-alarm: add FTM alarm driver")
Signed-off-by: Michael Walle
---
drivers/rtc/rtc-fsl-ftm-alarm.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/rtc/rtc-fsl-ftm-alarm.c b/drivers/rtc/rtc-fsl-ftm-alarm.c
index 57cc09d0a806..c0df49fb978c 100644
--- a/drivers/rtc/rtc-fsl-ftm-alarm.c
+++ b/drivers/rtc/rtc-fsl-ftm-alarm.c
@@ -310,6 +310,7 @@ static const struct of_device_id ftm_rtc_match[] = {
{ .compatible = "fsl,lx2160a-ftm-alarm", },
{ },
};
+MODULE_DEVICE_TABLE(of, ftm_rtc_match);
static const struct acpi_device_id ftm_imx_acpi_ids[] = {
{"NXP0014",},
--
2.20.1
[PATCH 2/2] arm64: dts: freescale: sl28: fix RGMII clock and voltage
During hardware validation it was noticed that the clock isn't
continuously enabled when there is no link. This is because the 125MHz
clock is derived from the internal PLL which seems to go into some kind
of power-down mode every once in a while. The LS1028A expects a contiuous
clock. Thus enable the PLL all the time.
Also, the RGMII pad voltage is wrong, it was configured to 2.5V (that is
the VDDH regulator). The correct voltage is 1.8V, i.e. the VDDIO
regulator.
This fix is for the freescale/fsl-ls1028a-kontron-sl28-var1.dts.
Fixes: 642856097c18 ("arm64: dts: freescale: sl28: add variant 1")
Signed-off-by: Michael Walle
---
.../arm64/boot/dts/freescale/fsl-ls1028a-kontron-sl28-var1.dts | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1028a-kontron-sl28-var1.dts
b/arch/arm64/boot/dts/freescale/fsl-ls1028a-kontron-sl28-var1.dts
index 6c309b97587d..e8d31279b7a3 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1028a-kontron-sl28-var1.dts
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a-kontron-sl28-var1.dts
@@ -46,7 +46,8 @@
eee-broken-100tx;
qca,clk-out-frequency = <12500>;
qca,clk-out-strength = ;
- vddio-supply = <>;
+ qca,keep-pll-enabled;
+ vddio-supply = <>;
vddio: vddio-regulator {
regulator-name = "VDDIO";
--
2.20.1
[PATCH 1/2] arm64: dts: freescale: sl28: fix RGMII clock and voltage
During hardware validation it was noticed that the clock isn't
continuously enabled when there is no link. This is because the 125MHz
clock is derived from the internal PLL which seems to go into some kind
of power-down mode every once in a while. The LS1028A expects a contiuous
clock. Thus enable the PLL all the time.
Also, the RGMII pad voltage is wrong. It was configured to 2.5V (that is
the VDDH regulator). The correct voltage is 1.8V, i.e. the VDDIO
regulator.
This fix is for the freescale/fsl-ls1028a-kontron-sl28-var4.dts.
Fixes: 815364d0424e ("arm64: dts: freescale: add Kontron sl28 support")
Signed-off-by: Michael Walle
---
.../boot/dts/freescale/fsl-ls1028a-kontron-sl28-var4.dts | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1028a-kontron-sl28-var4.dts
b/arch/arm64/boot/dts/freescale/fsl-ls1028a-kontron-sl28-var4.dts
index df212ed5bb94..e65d1c477e2c 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1028a-kontron-sl28-var4.dts
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a-kontron-sl28-var4.dts
@@ -31,11 +31,10 @@
reg = <0x4>;
eee-broken-1000t;
eee-broken-100tx;
-
qca,clk-out-frequency = <12500>;
qca,clk-out-strength = ;
-
- vddio-supply = <>;
+ qca,keep-pll-enabled;
+ vddio-supply = <>;
vddio: vddio-regulator {
regulator-name = "VDDIO";
--
2.20.1
[PATCH 0/2] arm64: dts: freescale: sl28: fix RGMII
This fixes the RGMII on the sl28 boards. While the network port was actually working it is still out-of-spec. Please note, that this is split into two patches because each one fixes a different commit. Michael Walle (2): arm64: dts: freescale: sl28: fix RGMII clock and voltage arm64: dts: freescale: sl28: fix RGMII clock and voltage .../boot/dts/freescale/fsl-ls1028a-kontron-sl28-var1.dts | 3 ++- .../boot/dts/freescale/fsl-ls1028a-kontron-sl28-var4.dts | 5 ++--- 2 files changed, 4 insertions(+), 4 deletions(-) -- 2.20.1
Re: [PATCH] mtd: spi-nor: macronix: Add block protection support to mx25u6435f
Hi Ikjoon, Am 2021-04-13 14:02, schrieb Ikjoon Jang: This patch adds block protection support to Macronix mx25u6432f and mx25u6435f. Two different chips share the same JEDEC ID while only mx25u6423f support section protections. And two chips have slightly different definitions of BP bits than generic (ST Micro) implementation. What is different compared to the current implementation? Could you give an example? So this patch defines a new spi_nor_locking_ops only for macronix until this could be merged into a generic swp implementation. TBH, I don't really like the code duplication and I'd presume that it won't ever be merged with the generic code. You also assume that both the WPSEL and T/B bit are 0, which might not be true. Please note that both are write-once, thus should only be read. See also: https://lore.kernel.org/linux-mtd/346332bf6ab0dd92b9ffd9e126b6b...@walle.cc/ -michael
[PATCH net-next v4 1/2] of: net: pass the dst buffer to of_get_mac_address()
of_get_mac_address() returns a "const void*" pointer to a MAC address.
Lately, support to fetch the MAC address by an NVMEM provider was added.
But this will only work with platform devices. It will not work with
PCI devices (e.g. of an integrated root complex) and esp. not with DSA
ports.
There is an of_* variant of the nvmem binding which works without
devices. The returned data of a nvmem_cell_read() has to be freed after
use. On the other hand the return of_get_mac_address() points to some
static data without a lifetime. The trick for now, was to allocate a
device resource managed buffer which is then returned. This will only
work if we have an actual device.
Change it, so that the caller of of_get_mac_address() has to supply a
buffer where the MAC address is written to. Unfortunately, this will
touch all drivers which use the of_get_mac_address().
Usually the code looks like:
const char *addr;
addr = of_get_mac_address(np);
if (!IS_ERR(addr))
ether_addr_copy(ndev->dev_addr, addr);
This can then be simply rewritten as:
of_get_mac_address(np, ndev->dev_addr);
Sometimes is_valid_ether_addr() is used to test the MAC address.
of_get_mac_address() already makes sure, it just returns a valid MAC
address. Thus we can just test its return code. But we have to be
careful if there are still other sources for the MAC address before the
of_get_mac_address(). In this case we have to keep the
is_valid_ether_addr() call.
The following coccinelle patch was used to convert common cases to the
new style. Afterwards, I've manually gone over the drivers and fixed the
return code variable: either used a new one or if one was already
available use that. Mansour Moufid, thanks for that coccinelle patch!
@a@
identifier x;
expression y, z;
@@
- x = of_get_mac_address(y);
+ x = of_get_mac_address(y, z);
<...
- ether_addr_copy(z, x);
...>
@@
identifier a.x;
@@
- if (<+... x ...+>) {}
@@
identifier a.x;
@@
if (<+... x ...+>) {
...
}
- else {}
@@
identifier a.x;
expression e;
@@
- if (<+... x ...+>@e)
- {}
- else
+ if (!(e))
{...}
@@
expression x, y, z;
@@
- x = of_get_mac_address(y, z);
+ of_get_mac_address(y, z);
... when != x
All drivers, except drivers/net/ethernet/aeroflex/greth.c, were
compile-time tested.
Suggested-by: Andrew Lunn
Signed-off-by: Michael Walle
---
arch/arm/mach-mvebu/kirkwood.c| 3 +-
arch/powerpc/sysdev/tsi108_dev.c | 5 +-
drivers/net/ethernet/aeroflex/greth.c | 6 +-
drivers/net/ethernet/allwinner/sun4i-emac.c | 10 +---
drivers/net/ethernet/altera/altera_tse_main.c | 7 +--
drivers/net/ethernet/arc/emac_main.c | 8 +--
drivers/net/ethernet/atheros/ag71xx.c | 7 +--
drivers/net/ethernet/broadcom/bcm4908_enet.c | 7 +--
drivers/net/ethernet/broadcom/bcmsysport.c| 7 +--
drivers/net/ethernet/broadcom/bgmac-bcma.c| 10 ++--
.../net/ethernet/broadcom/bgmac-platform.c| 11 ++--
drivers/net/ethernet/cadence/macb_main.c | 11 +---
.../net/ethernet/cavium/octeon/octeon_mgmt.c | 8 +--
.../net/ethernet/cavium/thunder/thunder_bgx.c | 5 +-
drivers/net/ethernet/davicom/dm9000.c | 10 ++--
drivers/net/ethernet/ethoc.c | 6 +-
drivers/net/ethernet/ezchip/nps_enet.c| 7 +--
drivers/net/ethernet/freescale/fec_main.c | 7 ++-
drivers/net/ethernet/freescale/fec_mpc52xx.c | 7 +--
drivers/net/ethernet/freescale/fman/mac.c | 9 +--
.../ethernet/freescale/fs_enet/fs_enet-main.c | 5 +-
drivers/net/ethernet/freescale/gianfar.c | 8 +--
drivers/net/ethernet/freescale/ucc_geth.c | 5 +-
drivers/net/ethernet/hisilicon/hisi_femac.c | 7 +--
drivers/net/ethernet/hisilicon/hix5hd2_gmac.c | 7 +--
drivers/net/ethernet/lantiq_xrx200.c | 7 +--
drivers/net/ethernet/marvell/mv643xx_eth.c| 5 +-
drivers/net/ethernet/marvell/mvneta.c | 6 +-
.../ethernet/marvell/prestera/prestera_main.c | 11 ++--
drivers/net/ethernet/marvell/pxa168_eth.c | 9 +--
drivers/net/ethernet/marvell/sky2.c | 8 +--
drivers/net/ethernet/mediatek/mtk_eth_soc.c | 11 ++--
drivers/net/ethernet/micrel/ks8851_common.c | 7 +--
drivers/net/ethernet/microchip/lan743x_main.c | 5 +-
drivers/net/ethernet/nxp/lpc_eth.c| 4 +-
drivers/net/ethernet/qualcomm/qca_spi.c | 10 +---
drivers/net/ethernet/qualcomm/qca_uart.c | 9 +--
drivers/net/ethernet/renesas/ravb_main.c | 12 ++--
drivers/net/ethernet/renesas/sh_eth.c | 5 +-
.../ethernet/samsung/sxgbe/sxgbe_platform.c | 13 ++--
drivers/net/ethernet/socionext/sni_ave.c | 10 +---
.../ethernet/stmicro/stmmac/dwmac-anarion.c | 2 +-
.../stmicro/stmmac/dwmac-dwc-qos-eth.c| 2 +-
.../ethernet/stmicro/stmmac/dwmac-generic.c | 2 +-
.../net/ethernet/stmicro/stmmac/dwmac-imx.c | 2 +-
.../stmicro/stmmac/dwmac-intel-plat.c | 2 +-
.../ethernet/stmicro/stmmac/dwmac-ipq806x.
[PATCH net-next v4 2/2] of: net: fix of_get_mac_addr_nvmem() for non-platform devices
of_get_mac_address() already supports fetching the MAC address by an
nvmem provider. But until now, it was just working for platform devices.
Esp. it was not working for DSA ports and PCI devices. It gets more
common that PCI devices have a device tree binding since SoCs contain
integrated root complexes.
Use the nvmem of_* binding to fetch the nvmem cells by a struct
device_node. We still have to try to read the cell by device first
because there might be a nvmem_cell_lookup associated with that device.
Signed-off-by: Michael Walle
---
drivers/of/of_net.c | 35 ++-
1 file changed, 30 insertions(+), 5 deletions(-)
diff --git a/drivers/of/of_net.c b/drivers/of/of_net.c
index cb77b774bf76..dbac3a172a11 100644
--- a/drivers/of/of_net.c
+++ b/drivers/of/of_net.c
@@ -11,6 +11,7 @@
#include
#include
#include
+#include
/**
* of_get_phy_mode - Get phy mode for given device_node
@@ -59,15 +60,39 @@ static int of_get_mac_addr(struct device_node *np, const
char *name, u8 *addr)
static int of_get_mac_addr_nvmem(struct device_node *np, u8 *addr)
{
struct platform_device *pdev = of_find_device_by_node(np);
+ struct nvmem_cell *cell;
+ const void *mac;
+ size_t len;
int ret;
- if (!pdev)
- return -ENODEV;
+ /* Try lookup by device first, there might be a nvmem_cell_lookup
+* associated with a given device.
+*/
+ if (pdev) {
+ ret = nvmem_get_mac_address(>dev, addr);
+ put_device(>dev);
+ return ret;
+ }
+
+ cell = of_nvmem_cell_get(np, "mac-address");
+ if (IS_ERR(cell))
+ return PTR_ERR(cell);
+
+ mac = nvmem_cell_read(cell, );
+ nvmem_cell_put(cell);
+
+ if (IS_ERR(mac))
+ return PTR_ERR(mac);
+
+ if (len != ETH_ALEN || !is_valid_ether_addr(mac)) {
+ kfree(mac);
+ return -EINVAL;
+ }
- ret = nvmem_get_mac_address(>dev, addr);
- put_device(>dev);
+ memcpy(addr, mac, ETH_ALEN);
+ kfree(mac);
- return ret;
+ return 0;
}
/**
--
2.20.1
[PATCH net-next v4 0/2] of: net: support non-platform devices in of_get_mac_address()
of_get_mac_address() is commonly used to fetch the MAC address from the device tree. It also supports reading it from a NVMEM provider. But the latter is only possible for platform devices, because only platform devices are searched for a matching device node. Add a second method to fetch the NVMEM cell by a device tree node instead of a "struct device". Moreover, the NVMEM subsystem will return dynamically allocated data which has to be freed after use. Currently, this is handled by allocating a device resource manged buffer to store the MAC address. of_get_mac_address() then returns a pointer to this buffer. Without a device, this trick is not possible anymore. Thus, change the of_get_mac_address() API to have the caller supply a buffer. It was considered to use the network device to attach the buffer to, but then the order matters and netdev_register() has to be called before of_get_mac_address(). No driver does it this way. changes since v3: - use memcpy() instead of ether_addr_copy() where appropriate. Sometimes the destination is on the stack, thus the 2 byte alignment requrement is not met. - fix "return PTR_ERR(mac_addr)" as found by Dan Carpenter - changed subject of patch 2/2, as suggested by Florian Fainelli changes since v2: - fixed of_get_mac_addr_nvmem() signature, which was accidentially fixed in patch 2/2 again changes since v1: - fixed stmmac_probe_config_dt() for !CONFIG_OF - added missing queue in patch subject Michael Walle (2): of: net: pass the dst buffer to of_get_mac_address() of: net: fix of_get_mac_addr_nvmem() for non-platform devices arch/arm/mach-mvebu/kirkwood.c| 3 +- arch/powerpc/sysdev/tsi108_dev.c | 5 +- drivers/net/ethernet/aeroflex/greth.c | 6 +- drivers/net/ethernet/allwinner/sun4i-emac.c | 10 +-- drivers/net/ethernet/altera/altera_tse_main.c | 7 +- drivers/net/ethernet/arc/emac_main.c | 8 +- drivers/net/ethernet/atheros/ag71xx.c | 7 +- drivers/net/ethernet/broadcom/bcm4908_enet.c | 7 +- drivers/net/ethernet/broadcom/bcmsysport.c| 7 +- drivers/net/ethernet/broadcom/bgmac-bcma.c| 10 +-- .../net/ethernet/broadcom/bgmac-platform.c| 11 ++- drivers/net/ethernet/cadence/macb_main.c | 11 +-- .../net/ethernet/cavium/octeon/octeon_mgmt.c | 8 +- .../net/ethernet/cavium/thunder/thunder_bgx.c | 5 +- drivers/net/ethernet/davicom/dm9000.c | 10 +-- drivers/net/ethernet/ethoc.c | 6 +- drivers/net/ethernet/ezchip/nps_enet.c| 7 +- drivers/net/ethernet/freescale/fec_main.c | 7 +- drivers/net/ethernet/freescale/fec_mpc52xx.c | 7 +- drivers/net/ethernet/freescale/fman/mac.c | 9 +- .../ethernet/freescale/fs_enet/fs_enet-main.c | 5 +- drivers/net/ethernet/freescale/gianfar.c | 8 +- drivers/net/ethernet/freescale/ucc_geth.c | 5 +- drivers/net/ethernet/hisilicon/hisi_femac.c | 7 +- drivers/net/ethernet/hisilicon/hix5hd2_gmac.c | 7 +- drivers/net/ethernet/lantiq_xrx200.c | 7 +- drivers/net/ethernet/marvell/mv643xx_eth.c| 5 +- drivers/net/ethernet/marvell/mvneta.c | 6 +- .../ethernet/marvell/prestera/prestera_main.c | 11 +-- drivers/net/ethernet/marvell/pxa168_eth.c | 9 +- drivers/net/ethernet/marvell/sky2.c | 8 +- drivers/net/ethernet/mediatek/mtk_eth_soc.c | 11 +-- drivers/net/ethernet/micrel/ks8851_common.c | 7 +- drivers/net/ethernet/microchip/lan743x_main.c | 5 +- drivers/net/ethernet/nxp/lpc_eth.c| 4 +- drivers/net/ethernet/qualcomm/qca_spi.c | 10 +-- drivers/net/ethernet/qualcomm/qca_uart.c | 9 +- drivers/net/ethernet/renesas/ravb_main.c | 12 +-- drivers/net/ethernet/renesas/sh_eth.c | 5 +- .../ethernet/samsung/sxgbe/sxgbe_platform.c | 13 +-- drivers/net/ethernet/socionext/sni_ave.c | 10 +-- .../ethernet/stmicro/stmmac/dwmac-anarion.c | 2 +- .../stmicro/stmmac/dwmac-dwc-qos-eth.c| 2 +- .../ethernet/stmicro/stmmac/dwmac-generic.c | 2 +- .../net/ethernet/stmicro/stmmac/dwmac-imx.c | 2 +- .../stmicro/stmmac/dwmac-intel-plat.c | 2 +- .../ethernet/stmicro/stmmac/dwmac-ipq806x.c | 2 +- .../ethernet/stmicro/stmmac/dwmac-lpc18xx.c | 2 +- .../ethernet/stmicro/stmmac/dwmac-mediatek.c | 2 +- .../net/ethernet/stmicro/stmmac/dwmac-meson.c | 2 +- .../ethernet/stmicro/stmmac/dwmac-meson8b.c | 2 +- .../net/ethernet/stmicro/stmmac/dwmac-oxnas.c | 2 +- .../stmicro/stmmac/dwmac-qcom-ethqos.c| 2 +- .../net/ethernet/stmicro/stmmac/dwmac-rk.c| 2 +- .../ethernet/stmicro/stmmac/dwmac-socfpga.c | 2 +- .../net/ethernet/stmicro/stmmac/dwmac-sti.c | 2 +- .../net/ethernet/stmicro/stmmac/dwmac-stm32.c | 2 +- .../net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 2 +- .../net/ethernet/stmicro/stmmac/dwmac-sunxi.c | 2 +- .../ethernet/stmicro/stmmac/dwmac-visconti.c | 2 +- drivers/net/ethernet/stmicro/stmmac/stmmac.h | 2
Re: [RFC PATCH 2/6] mtd: spi-nor: core: consolidate read op creation
Am 2021-03-11 20:12, schrieb Pratyush Yadav: Currently the spi_mem_op to read from the flash is used in two places: spi_nor_create_read_dirmap() and spi_nor_spimem_read_data(). In a later commit this number will increase to three. Instead of repeating the same code thrice, add a function that returns a template of the read op. The callers can then fill in the details like address, data length, data buffer location. Signed-off-by: Pratyush Yadav Reviewed-by: Michael Walle
Re: [PATCH] mtd: add OTP (one-time-programmable) erase ioctl
Hi Tudor, Am 2021-04-08 07:51, schrieb tudor.amba...@microchip.com: Would you please resend this patch, together with the mtd-utils and the SPI NOR patch in a single patch set? You'll help us all having all in a single place. This has already been picked-up: https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git/commit/?h=mtd/next=e3c1f1c92d6ede3cfa09d6a103d3d1c1ef645e35 Although, I didn't receive an email notice. -michael
[PATCH] mtd: spi-nor: implement OTP erase for Winbond and similar flashes
Winbond flashes with OTP support provide a command to erase the OTP
data. This might come in handy during development.
This was tested with a Winbond W25Q32JW on a LS1028A SoC with the
NXP FSPI controller.
Signed-off-by: Michael Walle
---
drivers/mtd/spi-nor/core.c| 4 +-
drivers/mtd/spi-nor/core.h| 4 ++
drivers/mtd/spi-nor/otp.c | 75 ++-
drivers/mtd/spi-nor/winbond.c | 1 +
4 files changed, 80 insertions(+), 4 deletions(-)
diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index 8cf3cf92129e..3f726a87e5a9 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -166,8 +166,8 @@ static int spi_nor_controller_ops_read_reg(struct spi_nor
*nor, u8 opcode,
return nor->controller_ops->read_reg(nor, opcode, buf, len);
}
-static int spi_nor_controller_ops_write_reg(struct spi_nor *nor, u8 opcode,
- const u8 *buf, size_t len)
+int spi_nor_controller_ops_write_reg(struct spi_nor *nor, u8 opcode,
+const u8 *buf, size_t len)
{
if (spi_nor_protocol_is_dtr(nor->reg_proto))
return -EOPNOTSUPP;
diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
index e9b6b2e76cdb..648e67bea0b2 100644
--- a/drivers/mtd/spi-nor/core.h
+++ b/drivers/mtd/spi-nor/core.h
@@ -213,6 +213,7 @@ struct spi_nor_otp_ops {
int (*read)(struct spi_nor *nor, loff_t addr, size_t len, u8 *buf);
int (*write)(struct spi_nor *nor, loff_t addr, size_t len, u8 *buf);
int (*lock)(struct spi_nor *nor, unsigned int region);
+ int (*erase)(struct spi_nor *nor, loff_t addr);
int (*is_locked)(struct spi_nor *nor, unsigned int region);
};
@@ -480,6 +481,8 @@ extern const struct spi_nor_manufacturer spi_nor_xmc;
void spi_nor_spimem_setup_op(const struct spi_nor *nor,
struct spi_mem_op *op,
const enum spi_nor_protocol proto);
+int spi_nor_controller_ops_write_reg(struct spi_nor *nor, u8 opcode,
+const u8 *buf, size_t len);
int spi_nor_write_enable(struct spi_nor *nor);
int spi_nor_write_disable(struct spi_nor *nor);
int spi_nor_set_4byte_addr_mode(struct spi_nor *nor, bool enable);
@@ -505,6 +508,7 @@ ssize_t spi_nor_write_data(struct spi_nor *nor, loff_t to,
size_t len,
int spi_nor_otp_read_secr(struct spi_nor *nor, loff_t addr, size_t len, u8
*buf);
int spi_nor_otp_write_secr(struct spi_nor *nor, loff_t addr, size_t len, u8
*buf);
+int spi_nor_otp_erase_secr(struct spi_nor *nor, loff_t addr);
int spi_nor_otp_lock_sr2(struct spi_nor *nor, unsigned int region);
int spi_nor_otp_is_locked_sr2(struct spi_nor *nor, unsigned int region);
diff --git a/drivers/mtd/spi-nor/otp.c b/drivers/mtd/spi-nor/otp.c
index 5021d40dffbf..ea7db4b354f7 100644
--- a/drivers/mtd/spi-nor/otp.c
+++ b/drivers/mtd/spi-nor/otp.c
@@ -8,6 +8,7 @@
#include
#include
#include
+#include
#include "core.h"
@@ -110,6 +111,50 @@ int spi_nor_otp_write_secr(struct spi_nor *nor, loff_t
addr, size_t len, u8 *buf
return ret ?: written;
}
+/**
+ * spi_nor_otp_erase_secr() - erase one OTP region
+ * @nor:pointer to 'struct spi_nor'
+ * @to: offset to write to
+ * @len:number of bytes to write
+ * @buf:pointer to src buffer
+ *
+ * Erase one OTP region by using the SPINOR_OP_ESECR commands. This method is
+ * used on GigaDevice and Winbond flashes.
+ *
+ * Return: 0 on success, -errno otherwise
+ */
+int spi_nor_otp_erase_secr(struct spi_nor *nor, loff_t addr)
+{
+ int ret;
+
+ ret = spi_nor_write_enable(nor);
+ if (ret)
+ return ret;
+
+ if (nor->spimem) {
+ struct spi_mem_op op =
+ SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_ESECR, 0),
+ SPI_MEM_OP_ADDR(3, addr, 0),
+ SPI_MEM_OP_NO_DUMMY,
+ SPI_MEM_OP_NO_DATA);
+
+ spi_nor_spimem_setup_op(nor, , nor->write_proto);
+
+ ret = spi_mem_exec_op(nor->spimem, );
+ } else {
+ nor->bouncebuf[2] = addr & 0xff;
+ nor->bouncebuf[1] = (addr >> 8) & 0xff;
+ nor->bouncebuf[0] = (addr >> 16) & 0xff;
+
+ ret = spi_nor_controller_ops_write_reg(nor, SPINOR_OP_ESECR,
+ nor->bouncebuf, 3);
+ }
+ if (ret)
+ return ret;
+
+ return spi_nor_wait_till_ready(nor);
+}
+
static int spi_nor_otp_lock_bit_cr(unsigned int region)
{
static const int lock_bits[] = { SR2_LB1, SR2_LB2, SR2_LB3 };
@@ -315,12 +360,14 @@ static int spi_nor_mtd_otp_write(struct mtd_info *mtd,
loff_t to, size_t len,
return spi_nor_mtd_otp_read_write(mtd, to, len, retlen, buf, t
Re: [PATCH net-next v3 1/2] of: net: pass the dst buffer to of_get_mac_address()
Am 2021-04-07 00:09, schrieb Michael Walle:
[..]
diff --git a/drivers/of/of_net.c b/drivers/of/of_net.c
index bc0a27de69d4..2d5d5e59aea5 100644
--- a/drivers/of/of_net.c
+++ b/drivers/of/of_net.c
@@ -45,42 +45,35 @@ int of_get_phy_mode(struct device_node *np,
phy_interface_t *interface)
}
EXPORT_SYMBOL_GPL(of_get_phy_mode);
-static const void *of_get_mac_addr(struct device_node *np, const char
*name)
+static int of_get_mac_addr(struct device_node *np, const char *name,
u8 *addr)
{
struct property *pp = of_find_property(np, name, NULL);
- if (pp && pp->length == ETH_ALEN && is_valid_ether_addr(pp->value))
- return pp->value;
- return NULL;
+ if (pp && pp->length == ETH_ALEN && is_valid_ether_addr(pp->value)) {
+ ether_addr_copy(addr, pp->value);
Mh, I guess this should rather be memcpy(addr, pp->value, ETH_ALEN)
because
ether_addr_copy() needs 2 byte aligned source and destination buffers.
-michael
[PATCH net-next v3 1/2] of: net: pass the dst buffer to of_get_mac_address()
of_get_mac_address() returns a "const void*" pointer to a MAC address.
Lately, support to fetch the MAC address by an NVMEM provider was added.
But this will only work with platform devices. It will not work with
PCI devices (e.g. of an integrated root complex) and esp. not with DSA
ports.
There is an of_* variant of the nvmem binding which works without
devices. The returned data of a nvmem_cell_read() has to be freed after
use. On the other hand the return of_get_mac_address() points to some
static data without a lifetime. The trick for now, was to allocate a
device resource managed buffer which is then returned. This will only
work if we have an actual device.
Change it, so that the caller of of_get_mac_address() has to supply a
buffer where the MAC address is written to. Unfortunately, this will
touch all drivers which use the of_get_mac_address().
Usually the code looks like:
const char *addr;
addr = of_get_mac_address(np);
if (!IS_ERR(addr))
ether_addr_copy(ndev->dev_addr, addr);
This can then be simply rewritten as:
of_get_mac_address(np, ndev->dev_addr);
Sometimes is_valid_ether_addr() is used to test the MAC address.
of_get_mac_address() already makes sure, it just returns a valid MAC
address. Thus we can just test its return code. But we have to be
careful if there are still other sources for the MAC address before the
of_get_mac_address(). In this case we have to keep the
is_valid_ether_addr() call.
The following coccinelle patch was used to convert common cases to the
new style. Afterwards, I've manually gone over the drivers and fixed the
return code variable: either used a new one or if one was already
available use that. Mansour Moufid, thanks for that coccinelle patch!
@a@
identifier x;
expression y, z;
@@
- x = of_get_mac_address(y);
+ x = of_get_mac_address(y, z);
<...
- ether_addr_copy(z, x);
...>
@@
identifier a.x;
@@
- if (<+... x ...+>) {}
@@
identifier a.x;
@@
if (<+... x ...+>) {
...
}
- else {}
@@
identifier a.x;
expression e;
@@
- if (<+... x ...+>@e)
- {}
- else
+ if (!(e))
{...}
@@
expression x, y, z;
@@
- x = of_get_mac_address(y, z);
+ of_get_mac_address(y, z);
... when != x
All drivers, except drivers/net/ethernet/aeroflex/greth.c, were
compile-time tested.
Suggested-by: Andrew Lunn
Signed-off-by: Michael Walle
---
arch/arm/mach-mvebu/kirkwood.c| 3 +-
arch/powerpc/sysdev/tsi108_dev.c | 5 +-
drivers/net/ethernet/aeroflex/greth.c | 6 +-
drivers/net/ethernet/allwinner/sun4i-emac.c | 10 +---
drivers/net/ethernet/altera/altera_tse_main.c | 7 +--
drivers/net/ethernet/arc/emac_main.c | 8 +--
drivers/net/ethernet/atheros/ag71xx.c | 7 +--
drivers/net/ethernet/broadcom/bcm4908_enet.c | 7 +--
drivers/net/ethernet/broadcom/bcmsysport.c| 7 +--
drivers/net/ethernet/broadcom/bgmac-bcma.c| 10 ++--
.../net/ethernet/broadcom/bgmac-platform.c| 11 ++--
drivers/net/ethernet/cadence/macb_main.c | 11 +---
.../net/ethernet/cavium/octeon/octeon_mgmt.c | 8 +--
.../net/ethernet/cavium/thunder/thunder_bgx.c | 5 +-
drivers/net/ethernet/davicom/dm9000.c | 10 ++--
drivers/net/ethernet/ethoc.c | 6 +-
drivers/net/ethernet/ezchip/nps_enet.c| 7 +--
drivers/net/ethernet/freescale/fec_main.c | 7 ++-
drivers/net/ethernet/freescale/fec_mpc52xx.c | 7 +--
drivers/net/ethernet/freescale/fman/mac.c | 9 +--
.../ethernet/freescale/fs_enet/fs_enet-main.c | 5 +-
drivers/net/ethernet/freescale/gianfar.c | 8 +--
drivers/net/ethernet/freescale/ucc_geth.c | 5 +-
drivers/net/ethernet/hisilicon/hisi_femac.c | 7 +--
drivers/net/ethernet/hisilicon/hix5hd2_gmac.c | 7 +--
drivers/net/ethernet/lantiq_xrx200.c | 7 +--
drivers/net/ethernet/marvell/mv643xx_eth.c| 5 +-
drivers/net/ethernet/marvell/mvneta.c | 6 +-
.../ethernet/marvell/prestera/prestera_main.c | 11 ++--
drivers/net/ethernet/marvell/pxa168_eth.c | 9 +--
drivers/net/ethernet/marvell/sky2.c | 8 +--
drivers/net/ethernet/mediatek/mtk_eth_soc.c | 11 ++--
drivers/net/ethernet/micrel/ks8851_common.c | 7 +--
drivers/net/ethernet/microchip/lan743x_main.c | 5 +-
drivers/net/ethernet/nxp/lpc_eth.c| 4 +-
drivers/net/ethernet/qualcomm/qca_spi.c | 10 +---
drivers/net/ethernet/qualcomm/qca_uart.c | 9 +--
drivers/net/ethernet/renesas/ravb_main.c | 12 ++--
drivers/net/ethernet/renesas/sh_eth.c | 5 +-
.../ethernet/samsung/sxgbe/sxgbe_platform.c | 13 ++--
drivers/net/ethernet/socionext/sni_ave.c | 10 +---
.../ethernet/stmicro/stmmac/dwmac-anarion.c | 2 +-
.../stmicro/stmmac/dwmac-dwc-qos-eth.c| 2 +-
.../ethernet/stmicro/stmmac/dwmac-generic.c | 2 +-
.../net/ethernet/stmicro/stmmac/dwmac-imx.c | 2 +-
.../stmicro/stmmac/dwmac-intel-plat.c | 2 +-
.../ethernet/stmicro/stmmac/dwmac-ipq806x.
[PATCH net-next v3 2/2] of: net: fix of_get_mac_addr_nvmem() for PCI and DSA nodes
of_get_mac_address() already supports fetching the MAC address by an
nvmem provider. But until now, it was just working for platform devices.
Esp. it was not working for DSA ports and PCI devices. It gets more
common that PCI devices have a device tree binding since SoCs contain
integrated root complexes.
Use the nvmem of_* binding to fetch the nvmem cells by a struct
device_node. We still have to try to read the cell by device first
because there might be a nvmem_cell_lookup associated with that device.
Signed-off-by: Michael Walle
---
Please note, that I've kept the nvmem_get_mac_address() which operates
on a device. The new of_get_mac_addr_nvmem() is almost identical and
there are no users of the former function right now, but it seems to be
the "newer" version to get the MAC address for a "struct device". Thus
I've kept it. Please advise, if I should kill it though.
drivers/of/of_net.c | 35 ++-
1 file changed, 30 insertions(+), 5 deletions(-)
diff --git a/drivers/of/of_net.c b/drivers/of/of_net.c
index 2d5d5e59aea5..2323c6063eaf 100644
--- a/drivers/of/of_net.c
+++ b/drivers/of/of_net.c
@@ -11,6 +11,7 @@
#include
#include
#include
+#include
/**
* of_get_phy_mode - Get phy mode for given device_node
@@ -59,15 +60,39 @@ static int of_get_mac_addr(struct device_node *np, const
char *name, u8 *addr)
static int of_get_mac_addr_nvmem(struct device_node *np, u8 *addr)
{
struct platform_device *pdev = of_find_device_by_node(np);
+ struct nvmem_cell *cell;
+ const void *mac;
+ size_t len;
int ret;
- if (!pdev)
- return -ENODEV;
+ /* Try lookup by device first, there might be a nvmem_cell_lookup
+* associated with a given device.
+*/
+ if (pdev) {
+ ret = nvmem_get_mac_address(>dev, addr);
+ put_device(>dev);
+ return ret;
+ }
+
+ cell = of_nvmem_cell_get(np, "mac-address");
+ if (IS_ERR(cell))
+ return PTR_ERR(cell);
+
+ mac = nvmem_cell_read(cell, );
+ nvmem_cell_put(cell);
+
+ if (IS_ERR(mac))
+ return PTR_ERR(mac);
+
+ if (len != ETH_ALEN || !is_valid_ether_addr(mac)) {
+ kfree(mac);
+ return -EINVAL;
+ }
- ret = nvmem_get_mac_address(>dev, addr);
- put_device(>dev);
+ ether_addr_copy(addr, mac);
+ kfree(mac);
- return ret;
+ return 0;
}
/**
--
2.20.1
[PATCH net-next v3 0/2] of: net: support non-platform devices in of_get_mac_address()
of_get_mac_address() is commonly used to fetch the MAC address from the device tree. It also supports reading it from a NVMEM provider. But the latter is only possible for platform devices, because only platform devices are searched for a matching device node. Add a second method to fetch the NVMEM cell by a device tree node instead of a "struct device". Moreover, the NVMEM subsystem will return dynamically allocated data which has to be freed after use. Currently, this is handled by allocating a device resource manged buffer to store the MAC address. of_get_mac_address() then returns a pointer to this buffer. Without a device, this trick is not possible anymore. Thus, change the of_get_mac_address() API to have the caller supply a buffer. It was considered to use the network device to attach the buffer to, but then the order matters and netdev_register() has to be called before of_get_mac_address(). No driver does it this way. changes since v2: - fixed of_get_mac_addr_nvmem() signature, which was accidentially fixed in patch 2/2 again changes since v1: - fixed stmmac_probe_config_dt() for !CONFIG_OF - added missing queue in patch subject Michael Walle (2): of: net: pass the dst buffer to of_get_mac_address() of: net: fix of_get_mac_addr_nvmem() for PCI and DSA nodes arch/arm/mach-mvebu/kirkwood.c| 3 +- arch/powerpc/sysdev/tsi108_dev.c | 5 +- drivers/net/ethernet/aeroflex/greth.c | 6 +- drivers/net/ethernet/allwinner/sun4i-emac.c | 10 +-- drivers/net/ethernet/altera/altera_tse_main.c | 7 +- drivers/net/ethernet/arc/emac_main.c | 8 +- drivers/net/ethernet/atheros/ag71xx.c | 7 +- drivers/net/ethernet/broadcom/bcm4908_enet.c | 7 +- drivers/net/ethernet/broadcom/bcmsysport.c| 7 +- drivers/net/ethernet/broadcom/bgmac-bcma.c| 10 +-- .../net/ethernet/broadcom/bgmac-platform.c| 11 ++- drivers/net/ethernet/cadence/macb_main.c | 11 +-- .../net/ethernet/cavium/octeon/octeon_mgmt.c | 8 +- .../net/ethernet/cavium/thunder/thunder_bgx.c | 5 +- drivers/net/ethernet/davicom/dm9000.c | 10 +-- drivers/net/ethernet/ethoc.c | 6 +- drivers/net/ethernet/ezchip/nps_enet.c| 7 +- drivers/net/ethernet/freescale/fec_main.c | 7 +- drivers/net/ethernet/freescale/fec_mpc52xx.c | 7 +- drivers/net/ethernet/freescale/fman/mac.c | 9 +- .../ethernet/freescale/fs_enet/fs_enet-main.c | 5 +- drivers/net/ethernet/freescale/gianfar.c | 8 +- drivers/net/ethernet/freescale/ucc_geth.c | 5 +- drivers/net/ethernet/hisilicon/hisi_femac.c | 7 +- drivers/net/ethernet/hisilicon/hix5hd2_gmac.c | 7 +- drivers/net/ethernet/lantiq_xrx200.c | 7 +- drivers/net/ethernet/marvell/mv643xx_eth.c| 5 +- drivers/net/ethernet/marvell/mvneta.c | 6 +- .../ethernet/marvell/prestera/prestera_main.c | 11 +-- drivers/net/ethernet/marvell/pxa168_eth.c | 9 +- drivers/net/ethernet/marvell/sky2.c | 8 +- drivers/net/ethernet/mediatek/mtk_eth_soc.c | 11 +-- drivers/net/ethernet/micrel/ks8851_common.c | 7 +- drivers/net/ethernet/microchip/lan743x_main.c | 5 +- drivers/net/ethernet/nxp/lpc_eth.c| 4 +- drivers/net/ethernet/qualcomm/qca_spi.c | 10 +-- drivers/net/ethernet/qualcomm/qca_uart.c | 9 +- drivers/net/ethernet/renesas/ravb_main.c | 12 +-- drivers/net/ethernet/renesas/sh_eth.c | 5 +- .../ethernet/samsung/sxgbe/sxgbe_platform.c | 13 +-- drivers/net/ethernet/socionext/sni_ave.c | 10 +-- .../ethernet/stmicro/stmmac/dwmac-anarion.c | 2 +- .../stmicro/stmmac/dwmac-dwc-qos-eth.c| 2 +- .../ethernet/stmicro/stmmac/dwmac-generic.c | 2 +- .../net/ethernet/stmicro/stmmac/dwmac-imx.c | 2 +- .../stmicro/stmmac/dwmac-intel-plat.c | 2 +- .../ethernet/stmicro/stmmac/dwmac-ipq806x.c | 2 +- .../ethernet/stmicro/stmmac/dwmac-lpc18xx.c | 2 +- .../ethernet/stmicro/stmmac/dwmac-mediatek.c | 2 +- .../net/ethernet/stmicro/stmmac/dwmac-meson.c | 2 +- .../ethernet/stmicro/stmmac/dwmac-meson8b.c | 2 +- .../net/ethernet/stmicro/stmmac/dwmac-oxnas.c | 2 +- .../stmicro/stmmac/dwmac-qcom-ethqos.c| 2 +- .../net/ethernet/stmicro/stmmac/dwmac-rk.c| 2 +- .../ethernet/stmicro/stmmac/dwmac-socfpga.c | 2 +- .../net/ethernet/stmicro/stmmac/dwmac-sti.c | 2 +- .../net/ethernet/stmicro/stmmac/dwmac-stm32.c | 2 +- .../net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 2 +- .../net/ethernet/stmicro/stmmac/dwmac-sunxi.c | 2 +- .../ethernet/stmicro/stmmac/dwmac-visconti.c | 2 +- drivers/net/ethernet/stmicro/stmmac/stmmac.h | 2 +- .../net/ethernet/stmicro/stmmac/stmmac_main.c | 4 +- .../ethernet/stmicro/stmmac/stmmac_platform.c | 14 +-- .../ethernet/stmicro/stmmac/stmmac_platform.h | 2 +- drivers/net/ethernet/ti/am65-cpsw-nuss.c | 19 ++--- drivers/net/ethernet/ti/cpsw.c| 7 +- drivers/net/ethernet/ti/
[PATCH net-next v2 2/2] of: net: fix of_get_mac_addr_nvmem() for PCI and DSA nodes
of_get_mac_address() already supports fetching the MAC address by an
nvmem provider. But until now, it was just working for platform devices.
Esp. it was not working for DSA ports and PCI devices. It gets more
common that PCI devices have a device tree binding since SoCs contain
integrated root complexes.
Use the nvmem of_* binding to fetch the nvmem cells by a struct
device_node. We still have to try to read the cell by device first
because there might be a nvmem_cell_lookup associated with that device.
Signed-off-by: Michael Walle
---
Please note, that I've kept the nvmem_get_mac_address() which operates
on a device. The new of_get_mac_addr_nvmem() is almost identical and
there are no users of the former function right now, but it seems to be
the "newer" version to get the MAC address for a "struct device". Thus
I've kept it. Please advise, if I should kill it though.
drivers/of/of_net.c | 37 +++--
1 file changed, 31 insertions(+), 6 deletions(-)
diff --git a/drivers/of/of_net.c b/drivers/of/of_net.c
index 2344ad7fff5e..2323c6063eaf 100644
--- a/drivers/of/of_net.c
+++ b/drivers/of/of_net.c
@@ -11,6 +11,7 @@
#include
#include
#include
+#include
/**
* of_get_phy_mode - Get phy mode for given device_node
@@ -56,18 +57,42 @@ static int of_get_mac_addr(struct device_node *np, const
char *name, u8 *addr)
return -ENODEV;
}
-static int of_get_mac_addr_nvmem(struct device_node *np, u8 addr)
+static int of_get_mac_addr_nvmem(struct device_node *np, u8 *addr)
{
struct platform_device *pdev = of_find_device_by_node(np);
+ struct nvmem_cell *cell;
+ const void *mac;
+ size_t len;
int ret;
- if (!pdev)
- return -ENODEV;
+ /* Try lookup by device first, there might be a nvmem_cell_lookup
+* associated with a given device.
+*/
+ if (pdev) {
+ ret = nvmem_get_mac_address(>dev, addr);
+ put_device(>dev);
+ return ret;
+ }
+
+ cell = of_nvmem_cell_get(np, "mac-address");
+ if (IS_ERR(cell))
+ return PTR_ERR(cell);
+
+ mac = nvmem_cell_read(cell, );
+ nvmem_cell_put(cell);
+
+ if (IS_ERR(mac))
+ return PTR_ERR(mac);
+
+ if (len != ETH_ALEN || !is_valid_ether_addr(mac)) {
+ kfree(mac);
+ return -EINVAL;
+ }
- ret = nvmem_get_mac_address(>dev, addr);
- put_device(>dev);
+ ether_addr_copy(addr, mac);
+ kfree(mac);
- return ret;
+ return 0;
}
/**
--
2.20.1
[PATCH net-next v2 1/2] of: net: pass the dst buffer to of_get_mac_address()
of_get_mac_address() returns a "const void*" pointer to a MAC address.
Lately, support to fetch the MAC address by an NVMEM provider was added.
But this will only work with platform devices. It will not work with
PCI devices (e.g. of an integrated root complex) and esp. not with DSA
ports.
There is an of_* variant of the nvmem binding which works without
devices. The returned data of a nvmem_cell_read() has to be freed after
use. On the other hand the return of_get_mac_address() points to some
static data without a lifetime. The trick for now, was to allocate a
device resource managed buffer which is then returned. This will only
work if we have an actual device.
Change it, so that the caller of of_get_mac_address() has to supply a
buffer where the MAC address is written to. Unfortunately, this will
touch all drivers which use the of_get_mac_address().
Usually the code looks like:
const char *addr;
addr = of_get_mac_address(np);
if (!IS_ERR(addr))
ether_addr_copy(ndev->dev_addr, addr);
This can then be simply rewritten as:
of_get_mac_address(np, ndev->dev_addr);
Sometimes is_valid_ether_addr() is used to test the MAC address.
of_get_mac_address() already makes sure, it just returns a valid MAC
address. Thus we can just test its return code. But we have to be
careful if there are still other sources for the MAC address before the
of_get_mac_address(). In this case we have to keep the
is_valid_ether_addr() call.
The following coccinelle patch was used to convert common cases to the
new style. Afterwards, I've manually gone over the drivers and fixed the
return code variable: either used a new one or if one was already
available use that. Mansour Moufid, thanks for that coccinelle patch!
@a@
identifier x;
expression y, z;
@@
- x = of_get_mac_address(y);
+ x = of_get_mac_address(y, z);
<...
- ether_addr_copy(z, x);
...>
@@
identifier a.x;
@@
- if (<+... x ...+>) {}
@@
identifier a.x;
@@
if (<+... x ...+>) {
...
}
- else {}
@@
identifier a.x;
expression e;
@@
- if (<+... x ...+>@e)
- {}
- else
+ if (!(e))
{...}
@@
expression x, y, z;
@@
- x = of_get_mac_address(y, z);
+ of_get_mac_address(y, z);
... when != x
All drivers, except drivers/net/ethernet/aeroflex/greth.c, were
compile-time tested.
Suggested-by: Andrew Lunn
Signed-off-by: Michael Walle
---
arch/arm/mach-mvebu/kirkwood.c| 3 +-
arch/powerpc/sysdev/tsi108_dev.c | 5 +-
drivers/net/ethernet/aeroflex/greth.c | 6 +-
drivers/net/ethernet/allwinner/sun4i-emac.c | 10 +---
drivers/net/ethernet/altera/altera_tse_main.c | 7 +--
drivers/net/ethernet/arc/emac_main.c | 8 +--
drivers/net/ethernet/atheros/ag71xx.c | 7 +--
drivers/net/ethernet/broadcom/bcm4908_enet.c | 7 +--
drivers/net/ethernet/broadcom/bcmsysport.c| 7 +--
drivers/net/ethernet/broadcom/bgmac-bcma.c| 10 ++--
.../net/ethernet/broadcom/bgmac-platform.c| 11 ++--
drivers/net/ethernet/cadence/macb_main.c | 11 +---
.../net/ethernet/cavium/octeon/octeon_mgmt.c | 8 +--
.../net/ethernet/cavium/thunder/thunder_bgx.c | 5 +-
drivers/net/ethernet/davicom/dm9000.c | 10 ++--
drivers/net/ethernet/ethoc.c | 6 +-
drivers/net/ethernet/ezchip/nps_enet.c| 7 +--
drivers/net/ethernet/freescale/fec_main.c | 7 ++-
drivers/net/ethernet/freescale/fec_mpc52xx.c | 7 +--
drivers/net/ethernet/freescale/fman/mac.c | 9 +--
.../ethernet/freescale/fs_enet/fs_enet-main.c | 5 +-
drivers/net/ethernet/freescale/gianfar.c | 8 +--
drivers/net/ethernet/freescale/ucc_geth.c | 5 +-
drivers/net/ethernet/hisilicon/hisi_femac.c | 7 +--
drivers/net/ethernet/hisilicon/hix5hd2_gmac.c | 7 +--
drivers/net/ethernet/lantiq_xrx200.c | 7 +--
drivers/net/ethernet/marvell/mv643xx_eth.c| 5 +-
drivers/net/ethernet/marvell/mvneta.c | 6 +-
.../ethernet/marvell/prestera/prestera_main.c | 11 ++--
drivers/net/ethernet/marvell/pxa168_eth.c | 9 +--
drivers/net/ethernet/marvell/sky2.c | 8 +--
drivers/net/ethernet/mediatek/mtk_eth_soc.c | 11 ++--
drivers/net/ethernet/micrel/ks8851_common.c | 7 +--
drivers/net/ethernet/microchip/lan743x_main.c | 5 +-
drivers/net/ethernet/nxp/lpc_eth.c| 4 +-
drivers/net/ethernet/qualcomm/qca_spi.c | 10 +---
drivers/net/ethernet/qualcomm/qca_uart.c | 9 +--
drivers/net/ethernet/renesas/ravb_main.c | 12 ++--
drivers/net/ethernet/renesas/sh_eth.c | 5 +-
.../ethernet/samsung/sxgbe/sxgbe_platform.c | 13 ++--
drivers/net/ethernet/socionext/sni_ave.c | 10 +---
.../ethernet/stmicro/stmmac/dwmac-anarion.c | 2 +-
.../stmicro/stmmac/dwmac-dwc-qos-eth.c| 2 +-
.../ethernet/stmicro/stmmac/dwmac-generic.c | 2 +-
.../net/ethernet/stmicro/stmmac/dwmac-imx.c | 2 +-
.../stmicro/stmmac/dwmac-intel-plat.c | 2 +-
.../ethernet/stmicro/stmmac/dwmac-ipq806x.
[PATCH net-next v2 0/2] of: net: support non-platform devices in of_get_mac_address()
of_get_mac_address() is commonly used to fetch the MAC address from the device tree. It also supports reading it from a NVMEM provider. But the latter is only possible for platform devices, because only platform devices are searched for a matching device node. Add a second method to fetch the NVMEM cell by a device tree node instead of a "struct device". Moreover, the NVMEM subsystem will return dynamically allocated data which has to be freed after use. Currently, this is handled by allocating a device resource manged buffer to store the MAC address. of_get_mac_address() then returns a pointer to this buffer. Without a device, this trick is not possible anymore. Thus, change the of_get_mac_address() API to have the caller supply a buffer. It was considered to use the network device to attach the buffer to, but then the order matters and netdev_register() has to be called before of_get_mac_address(). No driver does it this way. changes since v1: - fixed stmmac_probe_config_dt() for !CONFIG_OF - added missing queue in patch subject Michael Walle (2): of: net: pass the dst buffer to of_get_mac_address() of: net: fix of_get_mac_addr_nvmem() for PCI and DSA nodes arch/arm/mach-mvebu/kirkwood.c| 3 +- arch/powerpc/sysdev/tsi108_dev.c | 5 +- drivers/net/ethernet/aeroflex/greth.c | 6 +- drivers/net/ethernet/allwinner/sun4i-emac.c | 10 +-- drivers/net/ethernet/altera/altera_tse_main.c | 7 +- drivers/net/ethernet/arc/emac_main.c | 8 +- drivers/net/ethernet/atheros/ag71xx.c | 7 +- drivers/net/ethernet/broadcom/bcm4908_enet.c | 7 +- drivers/net/ethernet/broadcom/bcmsysport.c| 7 +- drivers/net/ethernet/broadcom/bgmac-bcma.c| 10 +-- .../net/ethernet/broadcom/bgmac-platform.c| 11 ++- drivers/net/ethernet/cadence/macb_main.c | 11 +-- .../net/ethernet/cavium/octeon/octeon_mgmt.c | 8 +- .../net/ethernet/cavium/thunder/thunder_bgx.c | 5 +- drivers/net/ethernet/davicom/dm9000.c | 10 +-- drivers/net/ethernet/ethoc.c | 6 +- drivers/net/ethernet/ezchip/nps_enet.c| 7 +- drivers/net/ethernet/freescale/fec_main.c | 7 +- drivers/net/ethernet/freescale/fec_mpc52xx.c | 7 +- drivers/net/ethernet/freescale/fman/mac.c | 9 +- .../ethernet/freescale/fs_enet/fs_enet-main.c | 5 +- drivers/net/ethernet/freescale/gianfar.c | 8 +- drivers/net/ethernet/freescale/ucc_geth.c | 5 +- drivers/net/ethernet/hisilicon/hisi_femac.c | 7 +- drivers/net/ethernet/hisilicon/hix5hd2_gmac.c | 7 +- drivers/net/ethernet/lantiq_xrx200.c | 7 +- drivers/net/ethernet/marvell/mv643xx_eth.c| 5 +- drivers/net/ethernet/marvell/mvneta.c | 6 +- .../ethernet/marvell/prestera/prestera_main.c | 11 +-- drivers/net/ethernet/marvell/pxa168_eth.c | 9 +- drivers/net/ethernet/marvell/sky2.c | 8 +- drivers/net/ethernet/mediatek/mtk_eth_soc.c | 11 +-- drivers/net/ethernet/micrel/ks8851_common.c | 7 +- drivers/net/ethernet/microchip/lan743x_main.c | 5 +- drivers/net/ethernet/nxp/lpc_eth.c| 4 +- drivers/net/ethernet/qualcomm/qca_spi.c | 10 +-- drivers/net/ethernet/qualcomm/qca_uart.c | 9 +- drivers/net/ethernet/renesas/ravb_main.c | 12 +-- drivers/net/ethernet/renesas/sh_eth.c | 5 +- .../ethernet/samsung/sxgbe/sxgbe_platform.c | 13 +-- drivers/net/ethernet/socionext/sni_ave.c | 10 +-- .../ethernet/stmicro/stmmac/dwmac-anarion.c | 2 +- .../stmicro/stmmac/dwmac-dwc-qos-eth.c| 2 +- .../ethernet/stmicro/stmmac/dwmac-generic.c | 2 +- .../net/ethernet/stmicro/stmmac/dwmac-imx.c | 2 +- .../stmicro/stmmac/dwmac-intel-plat.c | 2 +- .../ethernet/stmicro/stmmac/dwmac-ipq806x.c | 2 +- .../ethernet/stmicro/stmmac/dwmac-lpc18xx.c | 2 +- .../ethernet/stmicro/stmmac/dwmac-mediatek.c | 2 +- .../net/ethernet/stmicro/stmmac/dwmac-meson.c | 2 +- .../ethernet/stmicro/stmmac/dwmac-meson8b.c | 2 +- .../net/ethernet/stmicro/stmmac/dwmac-oxnas.c | 2 +- .../stmicro/stmmac/dwmac-qcom-ethqos.c| 2 +- .../net/ethernet/stmicro/stmmac/dwmac-rk.c| 2 +- .../ethernet/stmicro/stmmac/dwmac-socfpga.c | 2 +- .../net/ethernet/stmicro/stmmac/dwmac-sti.c | 2 +- .../net/ethernet/stmicro/stmmac/dwmac-stm32.c | 2 +- .../net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 2 +- .../net/ethernet/stmicro/stmmac/dwmac-sunxi.c | 2 +- .../ethernet/stmicro/stmmac/dwmac-visconti.c | 2 +- drivers/net/ethernet/stmicro/stmmac/stmmac.h | 2 +- .../net/ethernet/stmicro/stmmac/stmmac_main.c | 4 +- .../ethernet/stmicro/stmmac/stmmac_platform.c | 14 +-- .../ethernet/stmicro/stmmac/stmmac_platform.h | 2 +- drivers/net/ethernet/ti/am65-cpsw-nuss.c | 19 ++--- drivers/net/ethernet/ti/cpsw.c| 7 +- drivers/net/ethernet/ti/cpsw_new.c| 7 +- drivers/net/ethernet/ti/davinci_emac.c| 8 +- drivers/net/ethernet/ti/netcp_core.c
Re: [PATCH 2/2] mtd: spi-nor: add initial sysfs support
Hi, Am 2021-04-06 09:56, schrieb Vignesh Raghavendra: Hi, On 3/18/21 2:54 PM, Michael Walle wrote: Add support to show the name and JEDEC identifier as well as to dump the SFDP table. Not all flashes list their SFDP table contents in their datasheet. So having that is useful. It might also be helpful in bug reports from users. Sorry for the delay.. There is already debugfs support for dumping JEDEC ID [1]. Any reason to add sysfs entry as well? This is per mtd while the sfdp is per flash device. IMHO both should be at the same place. That brings up another question. Since SFDP dumps are more of a debug aid, should this be a debugfs entry rather than sysfs entry? And you're not the first one asking that. My argument was that the debugfs might not be available just when you need it. A developer could easily rebuild a kernel, but imagine some user with a COTS distro and some problems, then it is not that easy anymore. But thats your call to make. Note that sysfs entries are userspace ABIs just like syscalls and thus need to be documented in Documentation/ABI/testing/ or Documentation/ABI/stable. Thus need to be carefully designed compared to debugfs which are much more flexible. Ok. But I don't see a problem adding these read-only files /sfdp /name /jedec-id Do you? -michael
[PATCH 2/2] of: net: fix of_get_mac_addr_nvmem() for PCI and DSA nodes
of_get_mac_address() already supports fetching the MAC address by an
nvmem provider. But until now, it was just working for platform devices.
Esp. it was not working for DSA ports and PCI devices. It gets more
common that PCI devices have a device tree binding since SoCs contain
integrated root complexes.
Use the nvmem of_* binding to fetch the nvmem cells by a struct
device_node. We still have to try to read the cell by device first
because there might be a nvmem_cell_lookup associated with that device.
Signed-off-by: Michael Walle
---
Please note, that I've kept the nvmem_get_mac_address() which operates
on a device. The new of_get_mac_addr_nvmem() is almost identical and
there are no users of the former function right now, but it seems to be
the "newer" version to get the MAC address for a "struct device". Thus
I've kept it. Please advise, if I should kill it though.
drivers/of/of_net.c | 37 +++--
1 file changed, 31 insertions(+), 6 deletions(-)
diff --git a/drivers/of/of_net.c b/drivers/of/of_net.c
index 2344ad7fff5e..2323c6063eaf 100644
--- a/drivers/of/of_net.c
+++ b/drivers/of/of_net.c
@@ -11,6 +11,7 @@
#include
#include
#include
+#include
/**
* of_get_phy_mode - Get phy mode for given device_node
@@ -56,18 +57,42 @@ static int of_get_mac_addr(struct device_node *np, const
char *name, u8 *addr)
return -ENODEV;
}
-static int of_get_mac_addr_nvmem(struct device_node *np, u8 addr)
+static int of_get_mac_addr_nvmem(struct device_node *np, u8 *addr)
{
struct platform_device *pdev = of_find_device_by_node(np);
+ struct nvmem_cell *cell;
+ const void *mac;
+ size_t len;
int ret;
- if (!pdev)
- return -ENODEV;
+ /* Try lookup by device first, there might be a nvmem_cell_lookup
+* associated with a given device.
+*/
+ if (pdev) {
+ ret = nvmem_get_mac_address(>dev, addr);
+ put_device(>dev);
+ return ret;
+ }
+
+ cell = of_nvmem_cell_get(np, "mac-address");
+ if (IS_ERR(cell))
+ return PTR_ERR(cell);
+
+ mac = nvmem_cell_read(cell, );
+ nvmem_cell_put(cell);
+
+ if (IS_ERR(mac))
+ return PTR_ERR(mac);
+
+ if (len != ETH_ALEN || !is_valid_ether_addr(mac)) {
+ kfree(mac);
+ return -EINVAL;
+ }
- ret = nvmem_get_mac_address(>dev, addr);
- put_device(>dev);
+ ether_addr_copy(addr, mac);
+ kfree(mac);
- return ret;
+ return 0;
}
/**
--
2.20.1
[PATCH 1/2] of: net: pass the dst buffer to of_get_mac_address()
of_get_mac_address() returns a "const void*" pointer to a MAC address.
Lately, support to fetch the MAC address by an NVMEM provider was added.
But this will only work with platform devices. It will not work with
PCI devices (e.g. of an integrated root complex) and esp. not with DSA
ports.
There is an of_* variant of the nvmem binding which works without
devices. The returned data of a nvmem_cell_read() has to be freed after
use. On the other hand the return of_get_mac_address() points to some
static data without a lifetime. The trick for now, was to allocate a
device resource managed buffer which is then returned. This will only
work if we have an actual device.
Change it, so that the caller of of_get_mac_address() has to supply a
buffer where the MAC address is written to. Unfortunately, this will
touch all drivers which use the of_get_mac_address().
Usually the code looks like:
const char *addr;
addr = of_get_mac_address(np);
if (!IS_ERR(addr))
ether_addr_copy(ndev->dev_addr, addr);
This can then be simply rewritten as:
of_get_mac_address(np, ndev->dev_addr);
Sometimes is_valid_ether_addr() is used to test the MAC address.
of_get_mac_address() already makes sure, it just returns a valid MAC
address. Thus we can just test its return code. But we have to be
careful if there are still other sources for the MAC address before the
of_get_mac_address(). In this case we have to keep the
is_valid_ether_addr() call.
The following coccinelle patch was used to convert common cases to the
new style. Afterwards, I've manually gone over the drivers and fixed the
return code variable: either used a new one or if one was already
available use that. Mansour Moufid, thanks for that coccinelle patch!
@a@
identifier x;
expression y, z;
@@
- x = of_get_mac_address(y);
+ x = of_get_mac_address(y, z);
<...
- ether_addr_copy(z, x);
...>
@@
identifier a.x;
@@
- if (<+... x ...+>) {}
@@
identifier a.x;
@@
if (<+... x ...+>) {
...
}
- else {}
@@
identifier a.x;
expression e;
@@
- if (<+... x ...+>@e)
- {}
- else
+ if (!(e))
{...}
@@
expression x, y, z;
@@
- x = of_get_mac_address(y, z);
+ of_get_mac_address(y, z);
... when != x
All drivers, except drivers/net/ethernet/aeroflex/greth.c, were
compile-time tested.
Suggested-by: Andrew Lunn
Signed-off-by: Michael Walle
---
arch/arm/mach-mvebu/kirkwood.c| 3 +-
arch/powerpc/sysdev/tsi108_dev.c | 5 +-
drivers/net/ethernet/aeroflex/greth.c | 6 +-
drivers/net/ethernet/allwinner/sun4i-emac.c | 10 +---
drivers/net/ethernet/altera/altera_tse_main.c | 7 +--
drivers/net/ethernet/arc/emac_main.c | 8 +--
drivers/net/ethernet/atheros/ag71xx.c | 7 +--
drivers/net/ethernet/broadcom/bcm4908_enet.c | 7 +--
drivers/net/ethernet/broadcom/bcmsysport.c| 7 +--
drivers/net/ethernet/broadcom/bgmac-bcma.c| 10 ++--
.../net/ethernet/broadcom/bgmac-platform.c| 11 ++--
drivers/net/ethernet/cadence/macb_main.c | 11 +---
.../net/ethernet/cavium/octeon/octeon_mgmt.c | 8 +--
.../net/ethernet/cavium/thunder/thunder_bgx.c | 5 +-
drivers/net/ethernet/davicom/dm9000.c | 10 ++--
drivers/net/ethernet/ethoc.c | 6 +-
drivers/net/ethernet/ezchip/nps_enet.c| 7 +--
drivers/net/ethernet/freescale/fec_main.c | 7 ++-
drivers/net/ethernet/freescale/fec_mpc52xx.c | 7 +--
drivers/net/ethernet/freescale/fman/mac.c | 9 +--
.../ethernet/freescale/fs_enet/fs_enet-main.c | 5 +-
drivers/net/ethernet/freescale/gianfar.c | 8 +--
drivers/net/ethernet/freescale/ucc_geth.c | 5 +-
drivers/net/ethernet/hisilicon/hisi_femac.c | 7 +--
drivers/net/ethernet/hisilicon/hix5hd2_gmac.c | 7 +--
drivers/net/ethernet/lantiq_xrx200.c | 7 +--
drivers/net/ethernet/marvell/mv643xx_eth.c| 5 +-
drivers/net/ethernet/marvell/mvneta.c | 6 +-
.../ethernet/marvell/prestera/prestera_main.c | 11 ++--
drivers/net/ethernet/marvell/pxa168_eth.c | 9 +--
drivers/net/ethernet/marvell/sky2.c | 8 +--
drivers/net/ethernet/mediatek/mtk_eth_soc.c | 11 ++--
drivers/net/ethernet/micrel/ks8851_common.c | 7 +--
drivers/net/ethernet/microchip/lan743x_main.c | 5 +-
drivers/net/ethernet/nxp/lpc_eth.c| 4 +-
drivers/net/ethernet/qualcomm/qca_spi.c | 10 +---
drivers/net/ethernet/qualcomm/qca_uart.c | 9 +--
drivers/net/ethernet/renesas/ravb_main.c | 12 ++--
drivers/net/ethernet/renesas/sh_eth.c | 5 +-
.../ethernet/samsung/sxgbe/sxgbe_platform.c | 13 ++--
drivers/net/ethernet/socionext/sni_ave.c | 10 +---
.../ethernet/stmicro/stmmac/dwmac-anarion.c | 2 +-
.../stmicro/stmmac/dwmac-dwc-qos-eth.c| 2 +-
.../ethernet/stmicro/stmmac/dwmac-generic.c | 2 +-
.../net/ethernet/stmicro/stmmac/dwmac-imx.c | 2 +-
.../stmicro/stmmac/dwmac-intel-plat.c | 2 +-
.../ethernet/stmicro/stmmac/dwmac-ipq806x.
[PATCH 0/2] of: net: support non-platform devices in of_get_mac_address()
of_get_mac_address() is commonly used to fetch the MAC address from the device tree. It also supports reading it from a NVMEM provider. But the latter is only possible for platform devices, because only platform devices are searched for a matching device node. Add a second method to fetch the NVMEM cell by a device tree node instead of a "struct device". Moreover, the NVMEM subsystem will return dynamically allocated data which has to be freed after use. Currently, this is handled by allocating a device resource manged buffer to store the MAC address. of_get_mac_address() then returns a pointer to this buffer. Without a device, this trick is not possible anymore. Thus, change the of_get_mac_address() API to have the caller supply a buffer. It was considered to use the network device to attach the buffer to, but then the order matters and netdev_register() has to be called before of_get_mac_address(). No driver does it this way. Michael Walle (2): of: of_net: pass the dst buffer to of_get_mac_address() of: net: fix of_get_mac_address_nvmem() for PCI and DSA nodes arch/arm/mach-mvebu/kirkwood.c| 3 +- arch/powerpc/sysdev/tsi108_dev.c | 5 +- drivers/net/ethernet/aeroflex/greth.c | 6 +- drivers/net/ethernet/allwinner/sun4i-emac.c | 10 +-- drivers/net/ethernet/altera/altera_tse_main.c | 7 +- drivers/net/ethernet/arc/emac_main.c | 8 +- drivers/net/ethernet/atheros/ag71xx.c | 7 +- drivers/net/ethernet/broadcom/bcm4908_enet.c | 7 +- drivers/net/ethernet/broadcom/bcmsysport.c| 7 +- drivers/net/ethernet/broadcom/bgmac-bcma.c| 10 +-- .../net/ethernet/broadcom/bgmac-platform.c| 11 ++- drivers/net/ethernet/cadence/macb_main.c | 11 +-- .../net/ethernet/cavium/octeon/octeon_mgmt.c | 8 +- .../net/ethernet/cavium/thunder/thunder_bgx.c | 5 +- drivers/net/ethernet/davicom/dm9000.c | 10 +-- drivers/net/ethernet/ethoc.c | 6 +- drivers/net/ethernet/ezchip/nps_enet.c| 7 +- drivers/net/ethernet/freescale/fec_main.c | 7 +- drivers/net/ethernet/freescale/fec_mpc52xx.c | 7 +- drivers/net/ethernet/freescale/fman/mac.c | 9 +- .../ethernet/freescale/fs_enet/fs_enet-main.c | 5 +- drivers/net/ethernet/freescale/gianfar.c | 8 +- drivers/net/ethernet/freescale/ucc_geth.c | 5 +- drivers/net/ethernet/hisilicon/hisi_femac.c | 7 +- drivers/net/ethernet/hisilicon/hix5hd2_gmac.c | 7 +- drivers/net/ethernet/lantiq_xrx200.c | 7 +- drivers/net/ethernet/marvell/mv643xx_eth.c| 5 +- drivers/net/ethernet/marvell/mvneta.c | 6 +- .../ethernet/marvell/prestera/prestera_main.c | 11 +-- drivers/net/ethernet/marvell/pxa168_eth.c | 9 +- drivers/net/ethernet/marvell/sky2.c | 8 +- drivers/net/ethernet/mediatek/mtk_eth_soc.c | 11 +-- drivers/net/ethernet/micrel/ks8851_common.c | 7 +- drivers/net/ethernet/microchip/lan743x_main.c | 5 +- drivers/net/ethernet/nxp/lpc_eth.c| 4 +- drivers/net/ethernet/qualcomm/qca_spi.c | 10 +-- drivers/net/ethernet/qualcomm/qca_uart.c | 9 +- drivers/net/ethernet/renesas/ravb_main.c | 12 +-- drivers/net/ethernet/renesas/sh_eth.c | 5 +- .../ethernet/samsung/sxgbe/sxgbe_platform.c | 13 +-- drivers/net/ethernet/socionext/sni_ave.c | 10 +-- .../ethernet/stmicro/stmmac/dwmac-anarion.c | 2 +- .../stmicro/stmmac/dwmac-dwc-qos-eth.c| 2 +- .../ethernet/stmicro/stmmac/dwmac-generic.c | 2 +- .../net/ethernet/stmicro/stmmac/dwmac-imx.c | 2 +- .../stmicro/stmmac/dwmac-intel-plat.c | 2 +- .../ethernet/stmicro/stmmac/dwmac-ipq806x.c | 2 +- .../ethernet/stmicro/stmmac/dwmac-lpc18xx.c | 2 +- .../ethernet/stmicro/stmmac/dwmac-mediatek.c | 2 +- .../net/ethernet/stmicro/stmmac/dwmac-meson.c | 2 +- .../ethernet/stmicro/stmmac/dwmac-meson8b.c | 2 +- .../net/ethernet/stmicro/stmmac/dwmac-oxnas.c | 2 +- .../stmicro/stmmac/dwmac-qcom-ethqos.c| 2 +- .../net/ethernet/stmicro/stmmac/dwmac-rk.c| 2 +- .../ethernet/stmicro/stmmac/dwmac-socfpga.c | 2 +- .../net/ethernet/stmicro/stmmac/dwmac-sti.c | 2 +- .../net/ethernet/stmicro/stmmac/dwmac-stm32.c | 2 +- .../net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 2 +- .../net/ethernet/stmicro/stmmac/dwmac-sunxi.c | 2 +- .../ethernet/stmicro/stmmac/dwmac-visconti.c | 2 +- drivers/net/ethernet/stmicro/stmmac/stmmac.h | 2 +- .../net/ethernet/stmicro/stmmac/stmmac_main.c | 4 +- .../ethernet/stmicro/stmmac/stmmac_platform.c | 12 +-- .../ethernet/stmicro/stmmac/stmmac_platform.h | 2 +- drivers/net/ethernet/ti/am65-cpsw-nuss.c | 19 ++--- drivers/net/ethernet/ti/cpsw.c| 7 +- drivers/net/ethernet/ti/cpsw_new.c| 7 +- drivers/net/ethernet/ti/davinci_emac.c| 8 +- drivers/net/ethernet/ti/netcp_core.c | 7 +- drivers/net/ethernet/wiznet/w5100-spi.c | 8 +- drivers/net/ethernet/wizn
Re: [PATCH 1/2] mtd: spi-nor: sfdp: save a copy of the SFDP data
Am 2021-04-05 17:42, schrieb tudor.amba...@microchip.com:
On 4/5/21 6:07 PM, Michael Walle wrote:
EXTERNAL EMAIL: Do not click links or open attachments unless you know
the content is safe
Hi,
Am 2021-04-05 15:11, schrieb tudor.amba...@microchip.com:
On 3/18/21 11:24 AM, Michael Walle wrote:
EXTERNAL EMAIL: Do not click links or open attachments unless you
know
the content is safe
Due to possible mode switching to 8D-8D-8D, it might not be possible
to
read the SFDP after the initial probe. To be able to dump the SFDP
via
sysfs afterwards, make a complete copy of it.
Signed-off-by: Michael Walle
---
drivers/mtd/spi-nor/core.h | 10
drivers/mtd/spi-nor/sfdp.c | 49
+
include/linux/mtd/spi-nor.h | 3 +++
3 files changed, 62 insertions(+)
diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
index 4a3f7f150b5d..668f22011b1d 100644
--- a/drivers/mtd/spi-nor/core.h
+++ b/drivers/mtd/spi-nor/core.h
@@ -407,6 +407,16 @@ struct spi_nor_manufacturer {
const struct spi_nor_fixups *fixups;
};
+/**
+ * struct sfdp - SFDP data
+ * @num_dwords: number of entries in the dwords array
+ * @dwords: array of double words of the SFDP data
+ */
+struct sfdp {
+ size_t num_dwords;
+ u32 *dwords;
+};
+
/* Manufacturer drivers. */
extern const struct spi_nor_manufacturer spi_nor_atmel;
extern const struct spi_nor_manufacturer spi_nor_catalyst;
diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c
index 25142ec4737b..2b6c96e02532 100644
--- a/drivers/mtd/spi-nor/sfdp.c
+++ b/drivers/mtd/spi-nor/sfdp.c
@@ -16,6 +16,7 @@
(((p)->parameter_table_pointer[2] << 16) | \
((p)->parameter_table_pointer[1] << 8) | \
((p)->parameter_table_pointer[0] << 0))
+#define SFDP_PARAM_HEADER_PARAM_LEN(p) ((p)->length * 4)
#define SFDP_BFPT_ID 0xff00 /* Basic Flash Parameter
Table
*/
#define SFDP_SECTOR_MAP_ID 0xff81 /* Sector Map Table */
@@ -1263,6 +1264,8 @@ int spi_nor_parse_sfdp(struct spi_nor *nor,
struct sfdp_parameter_header *param_headers = NULL;
struct sfdp_header header;
struct device *dev = nor->dev;
+ struct sfdp *sfdp;
+ size_t sfdp_size;
size_t psize;
int i, err;
@@ -1285,6 +1288,9 @@ int spi_nor_parse_sfdp(struct spi_nor *nor,
bfpt_header->major != SFDP_JESD216_MAJOR)
return -EINVAL;
+ sfdp_size = SFDP_PARAM_HEADER_PTP(bfpt_header) +
+ SFDP_PARAM_HEADER_PARAM_LEN(bfpt_header);
+
/*
* Allocate memory then read all parameter headers with a
single
* Read SFDP command. These parameter headers will actually
be
parsed
@@ -1311,6 +1317,49 @@ int spi_nor_parse_sfdp(struct spi_nor *nor,
}
}
+ /*
+ * Cache the complete SFDP data. It is not (easily) possible
to fetch
+ * SFDP after probe time and we need it for the sysfs
access.
+ */
+ for (i = 0; i < header.nph; i++) {
+ param_header = _headers[i];
+ sfdp_size = max_t(size_t, sfdp_size,
+
SFDP_PARAM_HEADER_PTP(param_header)
+
+
SFDP_PARAM_HEADER_PARAM_LEN(param_header));
+ }
Michael, I like the idea of saving the SFDP data, but I think this
can
be
improved a little. For example, it is not mandatory for the tables to
be
continuous in memory, there can be some gaps between BFPT and SMPT
for
example,
thus we can improve the memory allocation logic.
I want to parse the SFDP as little as possible. Keep in mind, that
this
should
help to debug SFDP (errors). Therefore, I don't want to rely on the
SFDP
saying
"hey there is a hole, please skip it". Who knows if there is some
useful
data?
What kind of useful data? Do we care about data that doesn't follow the
jesd216
standard?
Yes because, it should be a raw dump of the SFDP data (of whatever
the flash vendor thinks is valid). You want to be able to debug
non-compliant SFDP data. Otherwise, this doesn't make any sense to
have it in the first place.
Also, we can make the saved sfdp
data table-agnostic so that we don't duplicate the reads in
parse_bfpt/smpt/4bait.
This falls into the same category as above. While it might be reused,
the
primary use case is to have the SFDP data available to a
developer/user.
Eg.
what will you do with some holes in the sysfs read()? Return zeros?
We don't have to have gaps in our internal buffer, we just allocate as
much
as we need and we write into our internal buffer just the sfdp tables,
without
the gaps.
There are two use cases:
(1) cache the data for the SFDP table parsing
(2) provide a raw dump of the SFDP
This patch targets (2). So first, you'd need to allocate multiple
buffers, then you'd have to combine them again for the raw SFDP dump
and finally you'd need to fill the gaps for the dump again. Because
w
Re: [PATCH 1/2] mtd: spi-nor: sfdp: save a copy of the SFDP data
Hi,
Am 2021-04-05 15:11, schrieb tudor.amba...@microchip.com:
On 3/18/21 11:24 AM, Michael Walle wrote:
EXTERNAL EMAIL: Do not click links or open attachments unless you know
the content is safe
Due to possible mode switching to 8D-8D-8D, it might not be possible
to
read the SFDP after the initial probe. To be able to dump the SFDP via
sysfs afterwards, make a complete copy of it.
Signed-off-by: Michael Walle
---
drivers/mtd/spi-nor/core.h | 10
drivers/mtd/spi-nor/sfdp.c | 49
+
include/linux/mtd/spi-nor.h | 3 +++
3 files changed, 62 insertions(+)
diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
index 4a3f7f150b5d..668f22011b1d 100644
--- a/drivers/mtd/spi-nor/core.h
+++ b/drivers/mtd/spi-nor/core.h
@@ -407,6 +407,16 @@ struct spi_nor_manufacturer {
const struct spi_nor_fixups *fixups;
};
+/**
+ * struct sfdp - SFDP data
+ * @num_dwords: number of entries in the dwords array
+ * @dwords: array of double words of the SFDP data
+ */
+struct sfdp {
+ size_t num_dwords;
+ u32 *dwords;
+};
+
/* Manufacturer drivers. */
extern const struct spi_nor_manufacturer spi_nor_atmel;
extern const struct spi_nor_manufacturer spi_nor_catalyst;
diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c
index 25142ec4737b..2b6c96e02532 100644
--- a/drivers/mtd/spi-nor/sfdp.c
+++ b/drivers/mtd/spi-nor/sfdp.c
@@ -16,6 +16,7 @@
(((p)->parameter_table_pointer[2] << 16) | \
((p)->parameter_table_pointer[1] << 8) | \
((p)->parameter_table_pointer[0] << 0))
+#define SFDP_PARAM_HEADER_PARAM_LEN(p) ((p)->length * 4)
#define SFDP_BFPT_ID 0xff00 /* Basic Flash Parameter Table
*/
#define SFDP_SECTOR_MAP_ID 0xff81 /* Sector Map Table */
@@ -1263,6 +1264,8 @@ int spi_nor_parse_sfdp(struct spi_nor *nor,
struct sfdp_parameter_header *param_headers = NULL;
struct sfdp_header header;
struct device *dev = nor->dev;
+ struct sfdp *sfdp;
+ size_t sfdp_size;
size_t psize;
int i, err;
@@ -1285,6 +1288,9 @@ int spi_nor_parse_sfdp(struct spi_nor *nor,
bfpt_header->major != SFDP_JESD216_MAJOR)
return -EINVAL;
+ sfdp_size = SFDP_PARAM_HEADER_PTP(bfpt_header) +
+ SFDP_PARAM_HEADER_PARAM_LEN(bfpt_header);
+
/*
* Allocate memory then read all parameter headers with a
single
* Read SFDP command. These parameter headers will actually be
parsed
@@ -1311,6 +1317,49 @@ int spi_nor_parse_sfdp(struct spi_nor *nor,
}
}
+ /*
+* Cache the complete SFDP data. It is not (easily) possible
to fetch
+* SFDP after probe time and we need it for the sysfs access.
+*/
+ for (i = 0; i < header.nph; i++) {
+ param_header = _headers[i];
+ sfdp_size = max_t(size_t, sfdp_size,
+ SFDP_PARAM_HEADER_PTP(param_header)
+
+
SFDP_PARAM_HEADER_PARAM_LEN(param_header));
+ }
Michael, I like the idea of saving the SFDP data, but I think this can
be
improved a little. For example, it is not mandatory for the tables to
be
continuous in memory, there can be some gaps between BFPT and SMPT for
example,
thus we can improve the memory allocation logic.
I want to parse the SFDP as little as possible. Keep in mind, that this
should
help to debug SFDP (errors). Therefore, I don't want to rely on the SFDP
saying
"hey there is a hole, please skip it". Who knows if there is some useful
data?
Also, we can make the saved sfdp
data table-agnostic so that we don't duplicate the reads in
parse_bfpt/smpt/4bait.
This falls into the same category as above. While it might be reused,
the
primary use case is to have the SFDP data available to a developer/user.
Eg.
what will you do with some holes in the sysfs read()? Return zeros?
FWIW I'm planning to refactor the read code so the cached values are
used.
-michael
Re: next/master bisection: baseline.login on kontron-kbox-a-230-ls
Hi Sahil, Am 2021-03-30 16:42, schrieb Sahil Malhotra: I tried the same on LS1028A-RDB board with 5.11 kernel and my submitted patch applied. Board booted up successfully: https://pastebin.com/15D91K5k I used OP-TEE from OP-TEE github repo: https://github.com/OP-TEE/optee_os with commit https://github.com/OP-TEE/optee_os/commit/d1447353d3f80ae49053b54f525a0a4cf5e4cde7 Which OP-TEE version you used for CI ? | Re: next/master bisection: baseline.login on kontron-kbox-a-230-ls That would be my board, which isn't a LS1028A-RDB and doesn't use optee at all. Hope this helps, to understand why this board crashed ;) -michael
Re: [RFC PATCH 3/4] dt-bindings: mtd: add OTP bindings
Hi Rob,
Am 2021-03-27 18:09, schrieb Rob Herring:
+spi {
+#address-cells = <1>;
+#size-cells = <0>;
+
+flash@0 {
+reg = <0>;
+compatible = "some,flash";
Soon (in linux-next, but off by default) this will be a warning for
undocumented compatible string. Use a real device.
Two questions:
(1) I guess this is also true for "PATCH 2/4", where you already added
your Reviewed-by?
(2) I'd add the "jedec,spi-nor" because, that is the one I target. But
before doing so, I'd need to add the otp subnode to the spi-nor
schema, correct? Otherwise, the schema validation will fail. Eg.
--- a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml
+++ b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml
@@ -9,6 +9,9 @@ title: SPI NOR flash ST M25Pxx (and similar) serial
flash chips
maintainers:
- Rob Herring
+allOf:
+ - $ref: "mtd.yaml#"
+
properties:
compatible:
oneOf:
@@ -82,6 +85,9 @@ patternProperties:
'^partition@':
type: object
+ "^otp(-[0-9]+)?$":
+type: object
+
additionalProperties: false
examples:
-michael
Re: [RFC PATCH 0/4] mtd: core: OTP nvmem provider support
Hi Srinivas,
Am 2021-03-30 11:42, schrieb Srinivas Kandagatla:
On 22/03/2021 18:19, Michael Walle wrote:
The goal is to fetch a (base) MAC address from the OTP region of a SPI
NOR
flash.
This is the first part, where I try to add the nvmem provider support
to
the MTD core.
I'm not sure about the device tree bindings. Consider the following
two
variants:
(1)
flash@0 {
..
otp {
compatible = "mtd-user-otp";
#address-cells = <1>;
#size-cells = <1>;
serial-number@0 {
reg = <0x0 0x8>;
};
};
};
(2)
flash@0 {
..
otp {
compatible = "mtd-user-otp";
#address-cells = <1>;
#size-cells = <1>;
some-useful-name {
compatible = "nvmem-cells";
serial-number@0 {
reg = <0x0 0x8>;
};
};
};
};
Both bindings use a subnode "opt[-N]". We cannot have the nvmem cells
as
children to the flash node because of the legacy partition binding.
(1) seems to be the form which is used almost everywhere in the
kernel.
That is, the nvmem cells are just children of the parent node.
(2) seem to be more natural, because there might also be other
properties
inside the otp subnode and might be more future-proof.
At the moment this patch implements (1).
Have you looked at this series[1], are you both trying to do the same
thing?
Yes, I've seen these, but they are for MTD partitions. OTP regions are
not
MTD partitions (and cannot be mapped to them).
-michael
[1]
https://lore.kernel.org/linux-mtd/20210312062830.20548-2-ansuels...@gmail.com/T/
--srini
Michael Walle (4):
nvmem: core: allow specifying of_node
dt-bindings: mtd: add YAML schema for the generic MTD bindings
dt-bindings: mtd: add OTP bindings
mtd: core: add OTP nvmem provider support
.../devicetree/bindings/mtd/common.txt| 16 +-
.../devicetree/bindings/mtd/mtd.yaml | 110 +
drivers/mtd/mtdcore.c | 149
++
drivers/nvmem/core.c | 4 +-
include/linux/mtd/mtd.h | 2 +
include/linux/nvmem-provider.h| 2 +
6 files changed, 267 insertions(+), 16 deletions(-)
create mode 100644 Documentation/devicetree/bindings/mtd/mtd.yaml
Re: [RFC PATCH 2/6] mtd: spi-nor: core: consolidate read op creation
Am 2021-03-11 20:12, schrieb Pratyush Yadav:
Currently the spi_mem_op to read from the flash is used in two places:
spi_nor_create_read_dirmap() and spi_nor_spimem_read_data(). In a later
commit this number will increase to three. Instead of repeating the
same
code thrice, add a function that returns a template of the read op. The
callers can then fill in the details like address, data length, data
buffer location.
Signed-off-by: Pratyush Yadav
---
drivers/mtd/spi-nor/core.c | 62 --
1 file changed, 32 insertions(+), 30 deletions(-)
diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index 4a315cb1c4db..8df009f0 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -183,6 +183,33 @@ static int spi_nor_controller_ops_erase(struct
spi_nor *nor, loff_t offs)
return nor->controller_ops->erase(nor, offs);
}
+/**
+ * spi_nor_spimem_get_read_op() - return a template for the spi_mem_op
used for
+ *reading data from the flash via
spi-mem.
+ * @nor:pointer to 'struct spi_nor'
+ *
+ * Return: A template of the 'struct spi_mem_op' for used for reading
data from
+ * the flash. The caller is expected to fill in the address, data
length, and
+ * the data buffer.
+ */
+static struct spi_mem_op spi_nor_spimem_get_read_op(struct spi_nor
*nor)
+{
+ struct spi_mem_op op =
+ SPI_MEM_OP(SPI_MEM_OP_CMD(nor->read_opcode, 0),
+ SPI_MEM_OP_ADDR(nor->addr_width, 0, 0),
+ SPI_MEM_OP_DUMMY(nor->read_dummy, 0),
+ SPI_MEM_OP_DATA_IN(1, NULL, 0));
+
+ spi_nor_spimem_setup_op(nor, , nor->read_proto);
+
+ /* convert the dummy cycles to the number of bytes */
+ op.dummy.nbytes = (nor->read_dummy * op.dummy.buswidth) / 8;
+ if (spi_nor_protocol_is_dtr(nor->read_proto))
+ op.dummy.nbytes *= 2;
+
+ return op;
+}
+
/**
* spi_nor_spimem_read_data() - read data from flash's memory region
via
* spi-mem
@@ -196,21 +223,14 @@ static int spi_nor_controller_ops_erase(struct
spi_nor *nor, loff_t offs)
static ssize_t spi_nor_spimem_read_data(struct spi_nor *nor, loff_t
from,
size_t len, u8 *buf)
{
- struct spi_mem_op op =
- SPI_MEM_OP(SPI_MEM_OP_CMD(nor->read_opcode, 0),
- SPI_MEM_OP_ADDR(nor->addr_width, from, 0),
- SPI_MEM_OP_DUMMY(nor->read_dummy, 0),
- SPI_MEM_OP_DATA_IN(len, buf, 0));
+ struct spi_mem_op op = spi_nor_spimem_get_read_op(nor);
bool usebouncebuf;
ssize_t nbytes;
int error;
- spi_nor_spimem_setup_op(nor, , nor->read_proto);
-
- /* convert the dummy cycles to the number of bytes */
- op.dummy.nbytes = (nor->read_dummy * op.dummy.buswidth) / 8;
- if (spi_nor_protocol_is_dtr(nor->read_proto))
- op.dummy.nbytes *= 2;
+ op.addr.val = from;
+ op.data.nbytes = len;
+ op.data.buf.in = buf;
usebouncebuf = spi_nor_spimem_bounce(nor, );
@@ -3581,28 +3601,10 @@ EXPORT_SYMBOL_GPL(spi_nor_scan);
static int spi_nor_create_read_dirmap(struct spi_nor *nor)
{
struct spi_mem_dirmap_info info = {
- .op_tmpl = SPI_MEM_OP(SPI_MEM_OP_CMD(nor->read_opcode, 0),
- SPI_MEM_OP_ADDR(nor->addr_width, 0, 0),
- SPI_MEM_OP_DUMMY(nor->read_dummy, 0),
- SPI_MEM_OP_DATA_IN(0, NULL, 0)),
+ .op_tmpl = spi_nor_spimem_get_read_op(nor),
.offset = 0,
.length = nor->mtd.size,
};
- struct spi_mem_op *op = _tmpl;
-
- spi_nor_spimem_setup_op(nor, op, nor->read_proto);
-
- /* convert the dummy cycles to the number of bytes */
- op->dummy.nbytes = (nor->read_dummy * op->dummy.buswidth) / 8;
- if (spi_nor_protocol_is_dtr(nor->read_proto))
- op->dummy.nbytes *= 2;
-
- /*
-* Since spi_nor_spimem_setup_op() only sets buswidth when the number
- * of data bytes is non-zero, the data buswidth won't be set here.
So,
-* do it explicitly.
-*/
- op->data.buswidth = spi_nor_get_protocol_data_nbits(nor->read_proto);
I guess this isn't needed anymore because spi_nor_spimem_get_read_op()
uses a
data length of 1, right?
nor->dirmap.rdesc = devm_spi_mem_dirmap_create(nor->dev, nor->spimem,
);
-michael
Re: [RFC PATCH 1/6] spi: spi-mem: Tell controller when device is ready for calibration
Hi Pratyush,
Am 2021-03-11 20:12, schrieb Pratyush Yadav:
Some controllers like the Cadence OSPI controller need to perform a
calibration sequence to operate at high clock speeds. This calibration
should happen after the flash is fully initialized otherwise the
calibration might happen in a different SPI mode from the one the flash
is finally set to. Add a hook that can be used to tell the controller
when the flash is ready for calibration. Whether calibration is needed
depends on the controller.
Signed-off-by: Pratyush Yadav
---
drivers/spi/spi-mem.c | 12
include/linux/spi/spi-mem.h | 8
2 files changed, 20 insertions(+)
diff --git a/drivers/spi/spi-mem.c b/drivers/spi/spi-mem.c
index dc713b0c3c4d..e2f05ad3f4dc 100644
--- a/drivers/spi/spi-mem.c
+++ b/drivers/spi/spi-mem.c
@@ -464,6 +464,18 @@ int spi_mem_adjust_op_size(struct spi_mem *mem,
struct spi_mem_op *op)
}
EXPORT_SYMBOL_GPL(spi_mem_adjust_op_size);
+int spi_mem_do_calibration(struct spi_mem *mem, struct spi_mem_op *op)
+{
+ struct spi_controller *ctlr = mem->spi->controller;
+
+ if (!ctlr->mem_ops || !ctlr->mem_ops->do_calibration)
+ return -EOPNOTSUPP;
+
+ ctlr->mem_ops->do_calibration(mem, op);
Can't a calibration fail?
+ return 0;
+}
+EXPORT_SYMBOL_GPL(spi_mem_do_calibration);
+
static ssize_t spi_mem_no_dirmap_read(struct spi_mem_dirmap_desc
*desc,
u64 offs, size_t len, void *buf)
{
diff --git a/include/linux/spi/spi-mem.h b/include/linux/spi/spi-mem.h
index 2b65c9edc34e..97a2d280f2d0 100644
--- a/include/linux/spi/spi-mem.h
+++ b/include/linux/spi/spi-mem.h
@@ -250,6 +250,12 @@ static inline void *spi_mem_get_drvdata(struct
spi_mem *mem)
* the currently mapped area), and the caller of
* spi_mem_dirmap_write() is responsible for calling it again in
* this case.
+ * @do_calibration: perform calibration needed for high SPI clock
speed
+ * operation. Should be called after the SPI memory device has
+ * been completely initialized. The op passed should contain
+ * a template for the read operation used for the device so
+ * the controller can decide what type of calibration is
+ * required for this type of read.
*
* This interface should be implemented by SPI controllers providing
an
* high-level interface to execute SPI memory operation, which is
usually the
@@ -274,6 +280,7 @@ struct spi_controller_mem_ops {
u64 offs, size_t len, void *buf);
ssize_t (*dirmap_write)(struct spi_mem_dirmap_desc *desc,
u64 offs, size_t len, const void *buf);
+ void (*do_calibration)(struct spi_mem *mem, struct spi_mem_op *op);
};
/**
@@ -346,6 +353,7 @@ bool spi_mem_dtr_supports_op(struct spi_mem *mem,
#endif /* CONFIG_SPI_MEM */
int spi_mem_adjust_op_size(struct spi_mem *mem, struct spi_mem_op
*op);
+int spi_mem_do_calibration(struct spi_mem *mem, struct spi_mem_op
*op);
bool spi_mem_supports_op(struct spi_mem *mem,
const struct spi_mem_op *op);
--
-michael
Re: [PATCH v2 1/2] mtd: spi-nor: sfdp: save a copy of the SFDP data
Am 23. März 2021 18:37:17 MEZ schrieb Pratyush Yadav : >On 23/03/21 03:31PM, Michael Walle wrote: >> Due to possible mode switching to 8D-8D-8D, it might not be possible >to >> read the SFDP after the initial probe. To be able to dump the SFDP >via >> sysfs afterwards, make a complete copy of it. >> >> Signed-off-by: Michael Walle > >Reviewed-by: Pratyush Yadav thanks for reviewing! -michael
[PATCH 1/2] arm64: dts: ls1028a: move rtc alias to individual boards
The aliases are board-specific and shouldn't be included in the common
SoC dtsi. Move them over to the boards.
Signed-off-by: Michael Walle
---
arch/arm64/boot/dts/freescale/fsl-ls1028a-kontron-sl28.dts | 1 +
arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dts | 1 +
arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts | 1 +
arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi | 4
4 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1028a-kontron-sl28.dts
b/arch/arm64/boot/dts/freescale/fsl-ls1028a-kontron-sl28.dts
index 576fc04cbce4..09f0b11d5705 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1028a-kontron-sl28.dts
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a-kontron-sl28.dts
@@ -25,6 +25,7 @@
spi1 =
mmc0 =
mmc1 =
+ rtc1 = _alarm0;
};
buttons0 {
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dts
b/arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dts
index fbcba9cb8503..bfd14b64567e 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dts
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dts
@@ -25,6 +25,7 @@
serial1 =
mmc0 =
mmc1 =
+ rtc1 = _alarm0;
};
chosen {
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts
b/arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts
index 41ae6e7675ba..9322c6ad8e4a 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts
@@ -21,6 +21,7 @@
serial1 =
mmc0 =
mmc1 =
+ rtc1 = _alarm0;
};
chosen {
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
b/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
index c1b33ac7d887..9506f0669ead 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
@@ -18,10 +18,6 @@
#address-cells = <2>;
#size-cells = <2>;
- aliases {
- rtc1 = _alarm0;
- };
-
cpus {
#address-cells = <1>;
#size-cells = <0>;
--
2.20.1
[PATCH 2/2] arm64: dts: fsl-ls1028a-kontron-sl28: add rtc0 alias
For completeness, add the rtc0 alias.
Signed-off-by: Michael Walle
---
arch/arm64/boot/dts/freescale/fsl-ls1028a-kontron-sl28.dts | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1028a-kontron-sl28.dts
b/arch/arm64/boot/dts/freescale/fsl-ls1028a-kontron-sl28.dts
index 09f0b11d5705..79a1224035c4 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1028a-kontron-sl28.dts
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a-kontron-sl28.dts
@@ -25,6 +25,7 @@
spi1 =
mmc0 =
mmc1 =
+ rtc0 =
rtc1 = _alarm0;
};
@@ -202,7 +203,7 @@
{
status = "okay";
- rtc@32 {
+ rtc: rtc@32 {
compatible = "microcrystal,rv8803";
reg = <0x32>;
};
--
2.20.1
[PATCH v2 2/2] mtd: spi-nor: add initial sysfs support
Add support to show the name and JEDEC identifier as well as to dump the
SFDP table. Not all flashes list their SFDP table contents in their
datasheet. So having that is useful. It might also be helpful in bug
reports from users.
Signed-off-by: Michael Walle
---
drivers/mtd/spi-nor/Makefile | 2 +-
drivers/mtd/spi-nor/core.c | 5 +++
drivers/mtd/spi-nor/core.h | 3 ++
drivers/mtd/spi-nor/sysfs.c | 86
4 files changed, 95 insertions(+), 1 deletion(-)
create mode 100644 drivers/mtd/spi-nor/sysfs.c
diff --git a/drivers/mtd/spi-nor/Makefile b/drivers/mtd/spi-nor/Makefile
index 653923896205..aff308f75987 100644
--- a/drivers/mtd/spi-nor/Makefile
+++ b/drivers/mtd/spi-nor/Makefile
@@ -1,6 +1,6 @@
# SPDX-License-Identifier: GPL-2.0
-spi-nor-objs := core.o sfdp.o
+spi-nor-objs := core.o sfdp.o sysfs.o
spi-nor-objs += atmel.o
spi-nor-objs += catalyst.o
spi-nor-objs += eon.o
diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index fbc34158a883..02523ddac612 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -3708,6 +3708,10 @@ static int spi_nor_probe(struct spi_mem *spimem)
if (ret)
return ret;
+ ret = spi_nor_sysfs_create(nor);
+ if (ret)
+ return ret;
+
return mtd_device_register(>mtd, data ? data->parts : NULL,
data ? data->nr_parts : 0);
}
@@ -3717,6 +3721,7 @@ static int spi_nor_remove(struct spi_mem *spimem)
struct spi_nor *nor = spi_mem_get_drvdata(spimem);
spi_nor_restore(nor);
+ spi_nor_sysfs_remove(nor);
/* Clean up MTD stuff. */
return mtd_device_unregister(>mtd);
diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
index 08d2469837da..599035200a03 100644
--- a/drivers/mtd/spi-nor/core.h
+++ b/drivers/mtd/spi-nor/core.h
@@ -486,4 +486,7 @@ static struct spi_nor __maybe_unused *mtd_to_spi_nor(struct
mtd_info *mtd)
return mtd->priv;
}
+int spi_nor_sysfs_create(struct spi_nor *nor);
+void spi_nor_sysfs_remove(struct spi_nor *nor);
+
#endif /* __LINUX_MTD_SPI_NOR_INTERNAL_H */
diff --git a/drivers/mtd/spi-nor/sysfs.c b/drivers/mtd/spi-nor/sysfs.c
new file mode 100644
index ..c62cc4d6bce6
--- /dev/null
+++ b/drivers/mtd/spi-nor/sysfs.c
@@ -0,0 +1,86 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include
+#include
+#include
+#include
+
+#include "core.h"
+
+static ssize_t name_show(struct device *dev,
+struct device_attribute *attr, char *buf)
+{
+ struct spi_device *spi = to_spi_device(dev);
+ struct spi_mem *spimem = spi_get_drvdata(spi);
+ struct spi_nor *nor = spi_mem_get_drvdata(spimem);
+
+ return sysfs_emit(buf, "%s\n", nor->info->name);
+}
+static DEVICE_ATTR_RO(name);
+
+static ssize_t jedec_id_show(struct device *dev,
+struct device_attribute *attr, char *buf)
+{
+ struct spi_device *spi = to_spi_device(dev);
+ struct spi_mem *spimem = spi_get_drvdata(spi);
+ struct spi_nor *nor = spi_mem_get_drvdata(spimem);
+
+ return sysfs_emit(buf, "%*phN\n", nor->info->id_len, nor->info->id);
+}
+static DEVICE_ATTR_RO(jedec_id);
+
+static struct attribute *spi_nor_sysfs_entries[] = {
+ _attr_name.attr,
+ _attr_jedec_id.attr,
+ NULL
+};
+
+static ssize_t sfdp_read(struct file *filp, struct kobject *kobj,
+struct bin_attribute *bin_attr, char *buf,
+loff_t off, size_t count)
+{
+ struct spi_device *spi = to_spi_device(kobj_to_dev(kobj));
+ struct spi_mem *spimem = spi_get_drvdata(spi);
+ struct spi_nor *nor = spi_mem_get_drvdata(spimem);
+ struct sfdp *sfdp = nor->sfdp;
+ size_t sfdp_size = sfdp->num_dwords * sizeof(*sfdp->dwords);
+
+ return memory_read_from_buffer(buf, count, , nor->sfdp->dwords,
+ sfdp_size);
+}
+static BIN_ATTR_RO(sfdp, 0);
+
+static struct bin_attribute *spi_nor_sysfs_bin_entries[] = {
+ _attr_sfdp,
+ NULL
+};
+
+static umode_t spi_nor_sysfs_is_bin_visible(struct kobject *kobj,
+ struct bin_attribute *attr, int n)
+{
+ struct spi_device *spi = to_spi_device(kobj_to_dev(kobj));
+ struct spi_mem *spimem = spi_get_drvdata(spi);
+ struct spi_nor *nor = spi_mem_get_drvdata(spimem);
+
+ if (attr == _attr_sfdp && !nor->sfdp)
+ return 0;
+
+ return 0444;
+}
+
+static struct attribute_group spi_nor_sysfs_attr_group = {
+ .name = NULL,
+ .is_bin_visible = spi_nor_sysfs_is_bin_visible,
+ .attrs = spi_nor_sysfs_entries,
+ .bin_attrs = spi_nor_sysfs_bin_entries,
+};
+
+int
[PATCH v2 0/2] mtd: spi-nor: support dumping sfdp tables
Add the possibility to dump the SFDP data of a flash device. More and more flash devices share the same flash ID and we need per device fixups. Usually, these fixups differentiate flashes by looking at differences in the SFDP data. Determining the difference is only possible if we have the SFDP data for all the flashes which share a flash ID. This will lay the foundation to dump the whole SFDP data of a flash device. This is even more important, because some datasheets doesn't even contain the SFDP data. Fixups for these kind of flashes are nearly impossible to do. I envision having a database of all the SFDP data for the flashes we support and make it a requirement to submit it when a new flash is added. This might or might not have legal implications. Thus I'd start with having that database private to the SPI NOR maintainers. Changes since v1: - use sysfs_emit() - add comment about the allocation of the sfdp dwords - free SFDP memory in the error path - use BIN_ATTR_RO(sfdp, 0) - use spi_nor_read_sfdp() Changes since RFC: - Don't read SFDP data after probe. The flash might already be switched to 8D-8D-8D mode. Instead, cache the SFDP data - add two sysfs files: jedec-id and name - change the file mode of the sfdp file from 0400 to 0444. There is no hardware access anymore. Michael Walle (2): mtd: spi-nor: sfdp: save a copy of the SFDP data mtd: spi-nor: add initial sysfs support drivers/mtd/spi-nor/Makefile | 2 +- drivers/mtd/spi-nor/core.c | 5 +++ drivers/mtd/spi-nor/core.h | 13 ++ drivers/mtd/spi-nor/sfdp.c | 58 drivers/mtd/spi-nor/sysfs.c | 86 include/linux/mtd/spi-nor.h | 2 + 6 files changed, 165 insertions(+), 1 deletion(-) create mode 100644 drivers/mtd/spi-nor/sysfs.c -- 2.20.1
[PATCH v2 1/2] mtd: spi-nor: sfdp: save a copy of the SFDP data
Due to possible mode switching to 8D-8D-8D, it might not be possible to
read the SFDP after the initial probe. To be able to dump the SFDP via
sysfs afterwards, make a complete copy of it.
Signed-off-by: Michael Walle
---
drivers/mtd/spi-nor/core.h | 10 +++
drivers/mtd/spi-nor/sfdp.c | 58 +
include/linux/mtd/spi-nor.h | 2 ++
3 files changed, 70 insertions(+)
diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
index db07832ee66c..08d2469837da 100644
--- a/drivers/mtd/spi-nor/core.h
+++ b/drivers/mtd/spi-nor/core.h
@@ -406,6 +406,16 @@ struct spi_nor_manufacturer {
const struct spi_nor_fixups *fixups;
};
+/**
+ * struct sfdp - SFDP data
+ * @num_dwords: number of entries in the dwords array
+ * @dwords: array of double words of the SFDP data
+ */
+struct sfdp {
+ size_t num_dwords;
+ u32 *dwords;
+};
+
/* Manufacturer drivers. */
extern const struct spi_nor_manufacturer spi_nor_atmel;
extern const struct spi_nor_manufacturer spi_nor_catalyst;
diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c
index 23c28e91f698..c500c2118a5d 100644
--- a/drivers/mtd/spi-nor/sfdp.c
+++ b/drivers/mtd/spi-nor/sfdp.c
@@ -16,6 +16,7 @@
(((p)->parameter_table_pointer[2] << 16) | \
((p)->parameter_table_pointer[1] << 8) | \
((p)->parameter_table_pointer[0] << 0))
+#define SFDP_PARAM_HEADER_PARAM_LEN(p) ((p)->length * 4)
#define SFDP_BFPT_ID 0xff00 /* Basic Flash Parameter Table */
#define SFDP_SECTOR_MAP_ID 0xff81 /* Sector Map Table */
@@ -1245,6 +1246,8 @@ int spi_nor_parse_sfdp(struct spi_nor *nor)
struct sfdp_parameter_header *param_headers = NULL;
struct sfdp_header header;
struct device *dev = nor->dev;
+ struct sfdp *sfdp;
+ size_t sfdp_size;
size_t psize;
int i, err;
@@ -1267,6 +1270,9 @@ int spi_nor_parse_sfdp(struct spi_nor *nor)
bfpt_header->major != SFDP_JESD216_MAJOR)
return -EINVAL;
+ sfdp_size = SFDP_PARAM_HEADER_PTP(bfpt_header) +
+ SFDP_PARAM_HEADER_PARAM_LEN(bfpt_header);
+
/*
* Allocate memory then read all parameter headers with a single
* Read SFDP command. These parameter headers will actually be parsed
@@ -1293,6 +1299,58 @@ int spi_nor_parse_sfdp(struct spi_nor *nor)
}
}
+ /*
+* Cache the complete SFDP data. It is not (easily) possible to fetch
+* SFDP after probe time and we need it for the sysfs access.
+*/
+ for (i = 0; i < header.nph; i++) {
+ param_header = _headers[i];
+ sfdp_size = max_t(size_t, sfdp_size,
+ SFDP_PARAM_HEADER_PTP(param_header) +
+ SFDP_PARAM_HEADER_PARAM_LEN(param_header));
+ }
+
+ /*
+* Limit the total size to a reasonable value to avoid allocating too
+* much memory just of because the flash returned some insane values.
+*/
+ if (sfdp_size > PAGE_SIZE) {
+ dev_dbg(dev, "SFDP data (%zu) too big, truncating\n",
+ sfdp_size);
+ sfdp_size = PAGE_SIZE;
+ }
+
+ sfdp = devm_kzalloc(dev, sizeof(*sfdp), GFP_KERNEL);
+ if (!sfdp) {
+ err = -ENOMEM;
+ goto exit;
+ }
+
+ /*
+* The SFDP is organized in chunks of DWORDs. Thus, in theory, the
+* sfdp_size should be a multiple of DWORDs. But in case a flash
+* is not spec compliant, make sure that we have enough space to store
+* the complete SFDP data.
+*/
+ sfdp->num_dwords = DIV_ROUND_UP(sfdp_size, sizeof(*sfdp->dwords));
+ sfdp->dwords = devm_kcalloc(dev, sfdp->num_dwords,
+ sizeof(*sfdp->dwords), GFP_KERNEL);
+ if (!sfdp->dwords) {
+ err = -ENOMEM;
+ devm_kfree(dev, sfdp);
+ goto exit;
+ }
+
+ err = spi_nor_read_sfdp(nor, 0, sfdp_size, sfdp->dwords);
+ if (err < 0) {
+ dev_dbg(dev, "failed to read SFDP data\n");
+ devm_kfree(dev, sfdp->dwords);
+ devm_kfree(dev, sfdp);
+ goto exit;
+ }
+
+ nor->sfdp = sfdp;
+
/*
* Check other parameter headers to get the latest revision of
* the basic flash parameter table.
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
index a0d572855444..2215e3565422 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -375,6 +375,7 @@ struct spi_nor_flash_parameter;
* @read_proto:the SPI protocol for read operations
* @write_proto: the SPI protocol for write operations
* @reg_proto: the SPI protocol for read_reg/write
Re: [PATCH 1/2] mtd: spi-nor: sfdp: save a copy of the SFDP data
Am 2021-03-22 19:42, schrieb Pratyush Yadav: On 22/03/21 04:32PM, Michael Walle wrote: Am 2021-03-22 15:21, schrieb Pratyush Yadav: > On 18/03/21 10:24AM, Michael Walle wrote: > > + > > + sfdp->num_dwords = DIV_ROUND_UP(sfdp_size, sizeof(*sfdp->dwords)); > > The SFDP spec says that Parameter Table Pointer should be DWORD aligned > and Parameter Table length is specified in number of DWORDs. So, > sfdp_size should always be a multiple of 4. Any SFDP table where this is > not true is an invalid one. > > Also, the spec says "Device behavior when the Read SFDP command crosses > the SFDP structure boundary is not defined". > > So I think this should be a check for alignment instead of a round-up. Well, that woundn't help for debugging. I.e. you also want the SFDP data in cases like this. IMHO we should try hard enough to actually get a reasonable dump. OTOH we also rely on the header and the pointers in the header. Any other ideas, but just to chicken out? Honestly, I don't think reading past the SFDP boundary would be too bad. It probably will just be some garbage data. But if you want to avoid that, you can always round down instead of up. Like I said, while the storage will be rounded up to a multiple of DWORDs, only sfdp_size is transferred. Thus it case a pointer is not DWORD aligned, we end up with zeros at the end. I'll add a comment. This way you will only miss the last DWORD at most. In either case, a warning should be printed so this problem can be brought to the user's attention. I was about to add a warning/debug message. But its the wrong place. It should really be checked in the for loop which iterates over the headers before parsing them. You could check sfdp_size but then two unaligned param pointers might cancel each other out. This can be a seperate patch, besides adding a warning, should there be any other things to do, e.g. stop parsing and error out? .. > > + goto exit; > > + } > > + > > + err = spi_nor_read_sfdp_dma_unsafe(nor, 0, sfdp_size, sfdp->dwords); Btw, this can be spi_nor_read_sfdp(). But I'm not sure, what this whole dma capable buffer should be. Is kmalloc(GFP_KERNEL) considered DMA safe? The buffer ends in spi_nor_read_data(), which is also called from mtdcore: spi_nor_read_sfdp() spi_nor_read_raw() spi_nor_read_data() mtd_read() mtd_read_oob() mtd_read_oob_std() spi_nor_read() spi_nor_read_data() Is the buffer passed from mtd_read() also DMA-safe? Doesn't the SPI drivers allocate DMA safe buffers if they need them? -michael
[RFC PATCH 4/4] mtd: core: add OTP nvmem provider support
Flash OTP regions can already be read via user space. Some boards have
their serial number or MAC addresses stored in the OTP regions. Add
support for them being a (read-only) nvmem provider.
The API to read the OTP data is already in place. It distinguishes
between factory and user OTP, thus there are up to two different
providers.
Signed-off-by: Michael Walle
---
drivers/mtd/mtdcore.c | 149
include/linux/mtd/mtd.h | 2 +
2 files changed, 151 insertions(+)
diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
index 432769caae04..300340973561 100644
--- a/drivers/mtd/mtdcore.c
+++ b/drivers/mtd/mtdcore.c
@@ -776,6 +776,147 @@ static void mtd_set_dev_defaults(struct mtd_info *mtd)
mutex_init(>master.chrdev_lock);
}
+static ssize_t mtd_otp_size(struct mtd_info *mtd, bool is_user)
+{
+ struct otp_info *info = kmalloc(PAGE_SIZE, GFP_KERNEL);
+ ssize_t size = 0;
+ unsigned int i;
+ size_t retlen;
+ int ret;
+
+ if (is_user)
+ ret = mtd_get_user_prot_info(mtd, PAGE_SIZE, , info);
+ else
+ ret = mtd_get_fact_prot_info(mtd, PAGE_SIZE, , info);
+ if (ret)
+ goto err;
+
+ for (i = 0; i < retlen / sizeof(*info); i++) {
+ size += info->length;
+ info++;
+ }
+
+ kfree(info);
+ return size;
+
+err:
+ kfree(info);
+ return ret;
+}
+
+static struct nvmem_device *mtd_otp_nvmem_register(struct mtd_info *mtd,
+ const char *name, int size,
+ nvmem_reg_read_t reg_read,
+ const char *compatible)
+{
+ struct nvmem_device *nvmem = NULL;
+ struct nvmem_config config = {};
+ struct device_node *np;
+
+ /* DT binding is optional */
+ np = of_get_compatible_child(mtd->dev.of_node, compatible);
+
+ /* OTP nvmem will be registered on the physical device */
+ config.dev = mtd->dev.parent;
+ config.name = name;
+ config.id = NVMEM_DEVID_NONE;
+ config.owner = THIS_MODULE;
+ config.type = NVMEM_TYPE_OTP;
+ config.root_only = true;
+ config.reg_read = reg_read;
+ config.size = size;
+ config.of_node = np;
+ config.priv = mtd;
+
+ nvmem = nvmem_register();
+ /* Just ignore if there is no NVMEM support in the kernel */
+ if (IS_ERR(nvmem) && PTR_ERR(nvmem) == -EOPNOTSUPP)
+ nvmem = NULL;
+
+ of_node_put(np);
+
+ return nvmem;
+}
+
+static int mtd_nvmem_user_otp_reg_read(void *priv, unsigned int offset,
+ void *val, size_t bytes)
+{
+ struct mtd_info *mtd = priv;
+ size_t retlen;
+ int ret;
+
+ ret = mtd_read_user_prot_reg(mtd, offset, bytes, , val);
+ if (ret)
+ return ret;
+
+ return retlen == bytes ? 0 : -EIO;
+}
+
+static int mtd_nvmem_fact_otp_reg_read(void *priv, unsigned int offset,
+ void *val, size_t bytes)
+{
+ struct mtd_info *mtd = priv;
+ size_t retlen;
+ int ret;
+
+ ret = mtd_read_fact_prot_reg(mtd, offset, bytes, , val);
+ if (ret)
+ return ret;
+
+ return retlen == bytes ? 0 : -EIO;
+}
+
+static int mtd_otp_nvmem_add(struct mtd_info *mtd)
+{
+ struct nvmem_device *nvmem;
+ ssize_t size;
+ int err;
+
+ if (mtd->_get_user_prot_info && mtd->_read_user_prot_reg) {
+ size = mtd_otp_size(mtd, true);
+ if (size < 0)
+ return size;
+
+ if (size > 0) {
+ nvmem = mtd_otp_nvmem_register(mtd, "user-otp", size,
+
mtd_nvmem_user_otp_reg_read,
+ "mtd-user-otp");
+ if (IS_ERR(nvmem)) {
+ dev_err(>dev, "Failed to register OTP
NVMEM device\n");
+ return PTR_ERR(nvmem);
+ }
+ mtd->otp_user_nvmem = nvmem;
+ }
+ }
+
+ if (mtd->_get_fact_prot_info && mtd->_read_fact_prot_reg) {
+ size = mtd_otp_size(mtd, false);
+ if (size < 0) {
+ err = size;
+ goto err;
+ }
+
+ if (size > 0) {
+ nvmem = mtd_otp_nvmem_register(mtd, "factory-otp", size,
+
mtd_nvmem_fact_otp_reg_read,
+ "mtd-factory-otp");
+ if (IS_ERR(nvmem)) {
+
[RFC PATCH 3/4] dt-bindings: mtd: add OTP bindings
Flash devices can have one-time-programmable regions. Add a nvmem
binding so they can be used as a nvmem provider.
Signed-off-by: Michael Walle
---
.../devicetree/bindings/mtd/mtd.yaml | 71 +++
1 file changed, 71 insertions(+)
diff --git a/Documentation/devicetree/bindings/mtd/mtd.yaml
b/Documentation/devicetree/bindings/mtd/mtd.yaml
index 321259aab0f6..2b852f91a6a9 100644
--- a/Documentation/devicetree/bindings/mtd/mtd.yaml
+++ b/Documentation/devicetree/bindings/mtd/mtd.yaml
@@ -21,6 +21,25 @@ properties:
based name) in order to ease flash device identification and/or
describe what they are used for.
+patternProperties:
+ "^otp(-[0-9]+)?":
+type: object
+$ref: ../nvmem/nvmem.yaml#
+
+description: |
+ An OTP memory region. Some flashes provide a one-time-programmable
+ memory whose content can either be programmed by a user or is already
+ pre-programmed by the factory. Some flashes might provide both.
+
+properties:
+ compatible:
+enum:
+ - mtd-user-otp
+ - mtd-factory-otp
+
+required:
+ - compatible
+
additionalProperties: true
examples:
@@ -36,4 +55,56 @@ examples:
};
};
+ - |
+spi {
+#address-cells = <1>;
+#size-cells = <0>;
+
+flash@0 {
+reg = <0>;
+compatible = "some,flash";
+
+otp {
+compatible = "mtd-user-otp";
+#address-cells = <1>;
+#size-cells = <1>;
+
+serial-number@0 {
+reg = <0 16>;
+};
+};
+};
+};
+
+ - |
+spi {
+#address-cells = <1>;
+#size-cells = <0>;
+
+flash@0 {
+reg = <0>;
+compatible = "some,flash";
+
+otp-1 {
+compatible = "mtd-factory-otp";
+#address-cells = <1>;
+#size-cells = <1>;
+
+electronic-serial-number@0 {
+reg = <0 8>;
+};
+};
+
+otp-2 {
+compatible = "mtd-user-otp";
+#address-cells = <1>;
+#size-cells = <1>;
+
+mac-address@0 {
+reg = <0 6>;
+};
+};
+};
+};
+
...
--
2.20.1
[RFC PATCH 0/4] mtd: core: OTP nvmem provider support
The goal is to fetch a (base) MAC address from the OTP region of a SPI NOR
flash.
This is the first part, where I try to add the nvmem provider support to
the MTD core.
I'm not sure about the device tree bindings. Consider the following two
variants:
(1)
flash@0 {
..
otp {
compatible = "mtd-user-otp";
#address-cells = <1>;
#size-cells = <1>;
serial-number@0 {
reg = <0x0 0x8>;
};
};
};
(2)
flash@0 {
..
otp {
compatible = "mtd-user-otp";
#address-cells = <1>;
#size-cells = <1>;
some-useful-name {
compatible = "nvmem-cells";
serial-number@0 {
reg = <0x0 0x8>;
};
};
};
};
Both bindings use a subnode "opt[-N]". We cannot have the nvmem cells as
children to the flash node because of the legacy partition binding.
(1) seems to be the form which is used almost everywhere in the kernel.
That is, the nvmem cells are just children of the parent node.
(2) seem to be more natural, because there might also be other properties
inside the otp subnode and might be more future-proof.
At the moment this patch implements (1).
Michael Walle (4):
nvmem: core: allow specifying of_node
dt-bindings: mtd: add YAML schema for the generic MTD bindings
dt-bindings: mtd: add OTP bindings
mtd: core: add OTP nvmem provider support
.../devicetree/bindings/mtd/common.txt| 16 +-
.../devicetree/bindings/mtd/mtd.yaml | 110 +
drivers/mtd/mtdcore.c | 149 ++
drivers/nvmem/core.c | 4 +-
include/linux/mtd/mtd.h | 2 +
include/linux/nvmem-provider.h| 2 +
6 files changed, 267 insertions(+), 16 deletions(-)
create mode 100644 Documentation/devicetree/bindings/mtd/mtd.yaml
--
2.20.1
[RFC PATCH 2/4] dt-bindings: mtd: add YAML schema for the generic MTD bindings
Convert MTD's common.txt to mtd.yaml.
Signed-off-by: Michael Walle
---
Btw, I've asked Miquel if I can add it as the maintainer.
.../devicetree/bindings/mtd/common.txt| 16 +---
.../devicetree/bindings/mtd/mtd.yaml | 39 +++
2 files changed, 40 insertions(+), 15 deletions(-)
create mode 100644 Documentation/devicetree/bindings/mtd/mtd.yaml
diff --git a/Documentation/devicetree/bindings/mtd/common.txt
b/Documentation/devicetree/bindings/mtd/common.txt
index fc068b923d7a..ae16f9ea8606 100644
--- a/Documentation/devicetree/bindings/mtd/common.txt
+++ b/Documentation/devicetree/bindings/mtd/common.txt
@@ -1,15 +1 @@
-* Common properties of all MTD devices
-
-Optional properties:
-- label: user-defined MTD device name. Can be used to assign user
- friendly names to MTD devices (instead of the flash model or flash
- controller based name) in order to ease flash device identification
- and/or describe what they are used for.
-
-Example:
-
- flash@0 {
- label = "System-firmware";
-
- /* flash type specific properties */
- };
+This file has been moved to mtd.yaml.
diff --git a/Documentation/devicetree/bindings/mtd/mtd.yaml
b/Documentation/devicetree/bindings/mtd/mtd.yaml
new file mode 100644
index ..321259aab0f6
--- /dev/null
+++ b/Documentation/devicetree/bindings/mtd/mtd.yaml
@@ -0,0 +1,39 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mtd/mtd.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: MTD (Memory Technology Device) Device Tree Bindings
+
+maintainers:
+ - Miquel Raynal
+ - Richard Weinberger
+
+properties:
+ $nodename:
+pattern: "^flash(@.*)?$"
+
+ label:
+description:
+ User-defined MTD device name. Can be used to assign user friendly
+ names to MTD devices (instead of the flash model or flash controller
+ based name) in order to ease flash device identification and/or
+ describe what they are used for.
+
+additionalProperties: true
+
+examples:
+ - |
+spi {
+#address-cells = <1>;
+#size-cells = <0>;
+
+flash@0 {
+reg = <0>;
+compatible = "some,flash";
+label = "System-firmware";
+};
+};
+
+...
--
2.20.1
[RFC PATCH 1/4] nvmem: core: allow specifying of_node
Until now, the of_node of the parent device is used. Some devices
provide more than just the nvmem provider. To avoid name space clashes,
add a way to allow specifying the nvmem cells in subnodes. Consider the
following example:
flash@0 {
compatible = "jedec,spi-nor";
partitions {
compatible = "fixed-partitions";
#address-cells = <1>;
#size-cells = <1>;
partition@0 {
reg = <0x00 0x01>;
};
};
otp {
compatible = "mtd-user-otp";
#address-cells = <1>;
#size-cells = <1>;
serial-number@0 {
reg = <0x0 0x8>;
};
};
};
There the nvmem provider might be the MTD partition or the OTP region of
the flash.
Add a new config->of_node parameter, which if set, will be used instead
of the parent's of_node.
Signed-off-by: Michael Walle
---
drivers/nvmem/core.c | 4 +++-
include/linux/nvmem-provider.h | 2 ++
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
index bca671ff4e54..62d363a399d3 100644
--- a/drivers/nvmem/core.c
+++ b/drivers/nvmem/core.c
@@ -789,7 +789,9 @@ struct nvmem_device *nvmem_register(const struct
nvmem_config *config)
nvmem->reg_write = config->reg_write;
nvmem->keepout = config->keepout;
nvmem->nkeepout = config->nkeepout;
- if (!config->no_of_node)
+ if (config->of_node)
+ nvmem->dev.of_node = config->of_node;
+ else if (!config->no_of_node)
nvmem->dev.of_node = config->dev->of_node;
switch (config->id) {
diff --git a/include/linux/nvmem-provider.h b/include/linux/nvmem-provider.h
index e162b757b6d5..471cb7b9e896 100644
--- a/include/linux/nvmem-provider.h
+++ b/include/linux/nvmem-provider.h
@@ -57,6 +57,7 @@ struct nvmem_keepout {
* @type: Type of the nvmem storage
* @read_only: Device is read-only.
* @root_only: Device is accessibly to root only.
+ * @of_node: If given, this will be used instead of the parent's of_node.
* @no_of_node:Device should not use the parent's of_node even if it's
!NULL.
* @reg_read: Callback to read data.
* @reg_write: Callback to write data.
@@ -86,6 +87,7 @@ struct nvmem_config {
enum nvmem_type type;
boolread_only;
boolroot_only;
+ struct device_node *of_node;
boolno_of_node;
nvmem_reg_read_treg_read;
nvmem_reg_write_t reg_write;
--
2.20.1
Re: [PATCH 1/2] mtd: spi-nor: sfdp: save a copy of the SFDP data
Am 2021-03-22 16:32, schrieb Michael Walle: + + sfdp->num_dwords = DIV_ROUND_UP(sfdp_size, sizeof(*sfdp->dwords)); The SFDP spec says that Parameter Table Pointer should be DWORD aligned and Parameter Table length is specified in number of DWORDs. So, sfdp_size should always be a multiple of 4. Any SFDP table where this is not true is an invalid one. Also, the spec says "Device behavior when the Read SFDP command crosses the SFDP structure boundary is not defined". So I think this should be a check for alignment instead of a round-up. Well, that woundn't help for debugging. I.e. you also want the SFDP data in cases like this. IMHO we should try hard enough to actually get a reasonable dump. OTOH we also rely on the header and the pointers in the header. Any other ideas, but just to chicken out? Oh, forgot to mention, sfdp_size is used to read the data. I just want to make sure, the allocated area is large enough. We shouldn't hit the undefined behavior by reading past the SFDP. Maybe that check should be part of the parsing code. -michael
Re: [PATCH 1/2] mtd: spi-nor: sfdp: save a copy of the SFDP data
Am 2021-03-22 15:21, schrieb Pratyush Yadav:
On 18/03/21 10:24AM, Michael Walle wrote:
Due to possible mode switching to 8D-8D-8D, it might not be possible
to
read the SFDP after the initial probe. To be able to dump the SFDP via
sysfs afterwards, make a complete copy of it.
Signed-off-by: Michael Walle
---
drivers/mtd/spi-nor/core.h | 10
drivers/mtd/spi-nor/sfdp.c | 49
+
include/linux/mtd/spi-nor.h | 3 +++
3 files changed, 62 insertions(+)
diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
index 4a3f7f150b5d..668f22011b1d 100644
--- a/drivers/mtd/spi-nor/core.h
+++ b/drivers/mtd/spi-nor/core.h
@@ -407,6 +407,16 @@ struct spi_nor_manufacturer {
const struct spi_nor_fixups *fixups;
};
+/**
+ * struct sfdp - SFDP data
+ * @num_dwords: number of entries in the dwords array
+ * @dwords: array of double words of the SFDP data
+ */
+struct sfdp {
+ size_t num_dwords;
+ u32 *dwords;
+};
+
/* Manufacturer drivers. */
extern const struct spi_nor_manufacturer spi_nor_atmel;
extern const struct spi_nor_manufacturer spi_nor_catalyst;
diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c
index 25142ec4737b..2b6c96e02532 100644
--- a/drivers/mtd/spi-nor/sfdp.c
+++ b/drivers/mtd/spi-nor/sfdp.c
@@ -16,6 +16,7 @@
(((p)->parameter_table_pointer[2] << 16) | \
((p)->parameter_table_pointer[1] << 8) | \
((p)->parameter_table_pointer[0] << 0))
+#define SFDP_PARAM_HEADER_PARAM_LEN(p) ((p)->length * 4)
#define SFDP_BFPT_ID 0xff00 /* Basic Flash Parameter Table */
#define SFDP_SECTOR_MAP_ID 0xff81 /* Sector Map Table */
@@ -1263,6 +1264,8 @@ int spi_nor_parse_sfdp(struct spi_nor *nor,
struct sfdp_parameter_header *param_headers = NULL;
struct sfdp_header header;
struct device *dev = nor->dev;
+ struct sfdp *sfdp;
+ size_t sfdp_size;
size_t psize;
int i, err;
@@ -1285,6 +1288,9 @@ int spi_nor_parse_sfdp(struct spi_nor *nor,
bfpt_header->major != SFDP_JESD216_MAJOR)
return -EINVAL;
+ sfdp_size = SFDP_PARAM_HEADER_PTP(bfpt_header) +
+ SFDP_PARAM_HEADER_PARAM_LEN(bfpt_header);
+
/*
* Allocate memory then read all parameter headers with a single
* Read SFDP command. These parameter headers will actually be
parsed
@@ -1311,6 +1317,49 @@ int spi_nor_parse_sfdp(struct spi_nor *nor,
}
}
+ /*
+ * Cache the complete SFDP data. It is not (easily) possible to
fetch
+* SFDP after probe time and we need it for the sysfs access.
+*/
+ for (i = 0; i < header.nph; i++) {
+ param_header = _headers[i];
+ sfdp_size = max_t(size_t, sfdp_size,
+ SFDP_PARAM_HEADER_PTP(param_header) +
+ SFDP_PARAM_HEADER_PARAM_LEN(param_header));
*sigh* If BFPT header was not made a part of the main SFDP header, two
"sfdp_size = ..." would not be needed, and we could do it all in one
place.
You can do that refactor if you're feeling like it, but I won't insist
on it.
I think that could be refactored when we also use the SFDP cache for
the remaining parsing.
+ }
+
+ /*
+ * Limit the total size to a reasonable value to avoid allocating
too
+ * much memory just of because the flash returned some insane
values.
+*/
+ if (sfdp_size > PAGE_SIZE) {
+ dev_dbg(dev, "SFDP data (%zu) too big, truncating\n",
+ sfdp_size);
+ sfdp_size = PAGE_SIZE;
Ok. 4K should be large enough for any reasonable SFDP table.
+ }
+
+ sfdp = devm_kzalloc(dev, sizeof(*sfdp), GFP_KERNEL);
+ if (!sfdp) {
+ err = -ENOMEM;
+ goto exit;
+ }
I assume you made nor->sfdp a pointer and not an embedded struct so it
can easily indicate if SFDP was found or not, correct?
Correct.
+
+ sfdp->num_dwords = DIV_ROUND_UP(sfdp_size, sizeof(*sfdp->dwords));
The SFDP spec says that Parameter Table Pointer should be DWORD aligned
and Parameter Table length is specified in number of DWORDs. So,
sfdp_size should always be a multiple of 4. Any SFDP table where this
is
not true is an invalid one.
Also, the spec says "Device behavior when the Read SFDP command crosses
the SFDP structure boundary is not defined".
So I think this should be a check for alignment instead of a round-up.
Well, that woundn't help for debugging. I.e. you also want the SFDP data
in cases like this. IMHO we should try hard enough to actually get a
reasonable dump.
OTOH we also rely on the header and the pointers in the header. Any
other ideas, but just to chicken out?
+ sfdp->dwords = devm_kcalloc(dev, sfdp->num_dwords,
+
Re: [PATCH 2/2] mtd: spi-nor: add initial sysfs support
Hi Pratyush,
Am 2021-03-22 15:43, schrieb Pratyush Yadav:
I have not worked with sysfs much so only giving this patch a quick
overview.
On 18/03/21 10:24AM, Michael Walle wrote:
Add support to show the name and JEDEC identifier as well as to dump
the
SFDP table. Not all flashes list their SFDP table contents in their
datasheet. So having that is useful. It might also be helpful in bug
reports from users.
The idea behind the sysfs module is also to have raw access to the SPI
NOR flash device registers, which can also be useful for debugging.
Your current patch does not add this feature. Why do you think it is
important enough to mention it?
It was part of my first (not submitted) version. I.e. you could also
read the status register(s). Comes in handy for debugging the locking
code, for example.
I can also remove that paragraph ;)
Signed-off-by: Michael Walle
---
drivers/mtd/spi-nor/Makefile | 2 +-
drivers/mtd/spi-nor/core.c | 5 +++
drivers/mtd/spi-nor/core.h | 3 ++
drivers/mtd/spi-nor/sysfs.c | 86
4 files changed, 95 insertions(+), 1 deletion(-)
create mode 100644 drivers/mtd/spi-nor/sysfs.c
diff --git a/drivers/mtd/spi-nor/Makefile
b/drivers/mtd/spi-nor/Makefile
index 653923896205..aff308f75987 100644
--- a/drivers/mtd/spi-nor/Makefile
+++ b/drivers/mtd/spi-nor/Makefile
@@ -1,6 +1,6 @@
# SPDX-License-Identifier: GPL-2.0
-spi-nor-objs := core.o sfdp.o
+spi-nor-objs := core.o sfdp.o sysfs.o
spi-nor-objs += atmel.o
spi-nor-objs += catalyst.o
spi-nor-objs += eon.o
diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index 4a315cb1c4db..2eaf4ba8c0f3 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -3707,6 +3707,10 @@ static int spi_nor_probe(struct spi_mem
*spimem)
if (ret)
return ret;
+ ret = spi_nor_sysfs_create(nor);
+ if (ret)
+ return ret;
+
return mtd_device_register(>mtd, data ? data->parts : NULL,
data ? data->nr_parts : 0);
}
@@ -3716,6 +3720,7 @@ static int spi_nor_remove(struct spi_mem
*spimem)
struct spi_nor *nor = spi_mem_get_drvdata(spimem);
spi_nor_restore(nor);
+ spi_nor_sysfs_remove(nor);
/* Clean up MTD stuff. */
return mtd_device_unregister(>mtd);
diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
index 668f22011b1d..dd592f7b62d1 100644
--- a/drivers/mtd/spi-nor/core.h
+++ b/drivers/mtd/spi-nor/core.h
@@ -488,4 +488,7 @@ static struct spi_nor __maybe_unused
*mtd_to_spi_nor(struct mtd_info *mtd)
return mtd->priv;
}
+int spi_nor_sysfs_create(struct spi_nor *nor);
+void spi_nor_sysfs_remove(struct spi_nor *nor);
+
#endif /* __LINUX_MTD_SPI_NOR_INTERNAL_H */
diff --git a/drivers/mtd/spi-nor/sysfs.c b/drivers/mtd/spi-nor/sysfs.c
new file mode 100644
index ..0de031e246c5
--- /dev/null
+++ b/drivers/mtd/spi-nor/sysfs.c
@@ -0,0 +1,86 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include
+#include
+#include
+#include
+
+#include "core.h"
+
+static ssize_t name_show(struct device *dev,
+struct device_attribute *attr, char *buf)
+{
+ struct spi_device *spi = to_spi_device(dev);
+ struct spi_mem *spimem = spi_get_drvdata(spi);
+ struct spi_nor *nor = spi_mem_get_drvdata(spimem);
+
+ return sprintf(buf, "%s\n", nor->info->name);
+}
+static DEVICE_ATTR_RO(name);
+
+static ssize_t jedec_id_show(struct device *dev,
+struct device_attribute *attr, char *buf)
+{
+ struct spi_device *spi = to_spi_device(dev);
+ struct spi_mem *spimem = spi_get_drvdata(spi);
+ struct spi_nor *nor = spi_mem_get_drvdata(spimem);
+
+ return sprintf(buf, "%*phN\n", nor->info->id_len, nor->info->id);
+}
+static DEVICE_ATTR_RO(jedec_id);
+
+static struct attribute *spi_nor_sysfs_entries[] = {
+ _attr_name.attr,
+ _attr_jedec_id.attr,
+ NULL
+};
+
+static ssize_t sfdp_read(struct file *filp, struct kobject *kobj,
+struct bin_attribute *bin_attr, char *buf,
+loff_t off, size_t count)
+{
+ struct spi_device *spi = to_spi_device(kobj_to_dev(kobj));
+ struct spi_mem *spimem = spi_get_drvdata(spi);
+ struct spi_nor *nor = spi_mem_get_drvdata(spimem);
+ struct sfdp *sfdp = nor->sfdp;
+ size_t sfdp_size = sfdp->num_dwords * sizeof(*sfdp->dwords);
Is a NULL check needed here? If the flash does not have a SFDP table
nor->sfdp will be NULL.
That can't happen because..
+
+ return memory_read_from_buffer(buf, count, , nor->sfdp->dwords,
+ sfdp_size);
+}
+static BIN_ATTR_RO(sfdp, PAGE_SIZE);
+
+static struct bin_attribute *spi_n
Re: [PATCH] arm64: dts: ls1028a: fix optee node
Am 2021-03-22 14:04, schrieb Sahil Malhotra: Sahil, if you like you can pick it up to enable the nodes for your ls1028a boards. Michael, If we enable the optee node like this, will this gets resolved ? I don't know what you mean. This was a fix for the initial patch. So, I guess the answer is yes it will not break my board if you don't enable optee globally, but just for your board. -michael
Re: [PATCH] arm64: dts: ls1028a: fix optee node
Hi Sahil, Am 2021-03-22 12:33, schrieb Sahil Malhotra: Thanks for the fix, and currently we support optee only on ls1028a-rdb boards. Does enabling the optee node this way, solves the issue ? What do you mean? Please note, that Shawn already reverted your commit. Therefore, I suggest you make a new commit where you enable optee only for the ls1028a-rdb board. -michael
Re: [PATCH v3 1/2] mtd: spi-nor: Move Software Write Protection logic out of the core
Am 2021-03-22 08:51, schrieb Tudor Ambarus: It makes the core file a bit smaller and provides better separation between the Software Write Protection features and the core logic. All the next generic software write protection features (e.g. Individual Block Protection) will reside in swp.c. Signed-off-by: Tudor Ambarus Reviewed-by: Michael Walle -michael
Re: [PATCH v3 2/2] mtd: spi-nor: swp: Improve code around spi_nor_check_lock_status_sr()
Am 2021-03-22 08:51, schrieb Tudor Ambarus: - bool return value for spi_nor_check_lock_status_sr(), gets rid of the return 1, - introduce temporary variables for better readability. Suggested-by: Joe Perches Signed-off-by: Tudor Ambarus --- Reviewed-by: Michael Walle -michael
[PATCH v5 2/3] mtd: spi-nor: implement OTP support for Winbond and similar flashes
Use the new OTP ops to implement OTP access on Winbond flashes. Most
Winbond flashes provides up to four different OTP regions ("Security
Registers").
Winbond devices use a special opcode to read and write to the OTP
regions, just like the RDSFDP opcode. In fact, it seems that the
(undocumented) first OTP area of the newer flashes is the actual SFDP
table.
On a side note, Winbond devices also allow erasing the OTP regions as
long as the area isn't locked down.
Signed-off-by: Michael Walle
---
drivers/mtd/spi-nor/core.c | 2 +-
drivers/mtd/spi-nor/core.h | 6 ++
drivers/mtd/spi-nor/otp.c | 164
include/linux/mtd/spi-nor.h | 8 ++
4 files changed, 179 insertions(+), 1 deletion(-)
diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index 5e8218e53ec7..3e6fbc9d53ef 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -1034,7 +1034,7 @@ static int spi_nor_write_16bit_sr_and_check(struct
spi_nor *nor, u8 sr1)
*
* Return: 0 on success, -errno otherwise.
*/
-static int spi_nor_write_16bit_cr_and_check(struct spi_nor *nor, u8 cr)
+int spi_nor_write_16bit_cr_and_check(struct spi_nor *nor, u8 cr)
{
int ret;
u8 *sr_cr = nor->bouncebuf;
diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
index 18521603b012..8b1f900e0e0a 100644
--- a/drivers/mtd/spi-nor/core.h
+++ b/drivers/mtd/spi-nor/core.h
@@ -495,6 +495,7 @@ int spi_nor_read_sr(struct spi_nor *nor, u8 *sr);
int spi_nor_read_cr(struct spi_nor *nor, u8 *cr);
int spi_nor_write_sr(struct spi_nor *nor, const u8 *sr, size_t len);
int spi_nor_write_sr_and_check(struct spi_nor *nor, u8 sr1);
+int spi_nor_write_16bit_cr_and_check(struct spi_nor *nor, u8 cr);
int spi_nor_xread_sr(struct spi_nor *nor, u8 *sr);
ssize_t spi_nor_read_data(struct spi_nor *nor, loff_t from, size_t len,
@@ -502,6 +503,11 @@ ssize_t spi_nor_read_data(struct spi_nor *nor, loff_t
from, size_t len,
ssize_t spi_nor_write_data(struct spi_nor *nor, loff_t to, size_t len,
const u8 *buf);
+int spi_nor_otp_read_secr(struct spi_nor *nor, loff_t addr, size_t len, u8
*buf);
+int spi_nor_otp_write_secr(struct spi_nor *nor, loff_t addr, size_t len, u8
*buf);
+int spi_nor_otp_lock_sr2(struct spi_nor *nor, unsigned int region);
+int spi_nor_otp_is_locked_sr2(struct spi_nor *nor, unsigned int region);
+
int spi_nor_hwcaps_read2cmd(u32 hwcaps);
u8 spi_nor_convert_3to4_read(u8 opcode);
void spi_nor_set_read_settings(struct spi_nor_read_command *read,
diff --git a/drivers/mtd/spi-nor/otp.c b/drivers/mtd/spi-nor/otp.c
index bc580a386883..40fa31d8fe38 100644
--- a/drivers/mtd/spi-nor/otp.c
+++ b/drivers/mtd/spi-nor/otp.c
@@ -14,6 +14,170 @@
#define spi_nor_otp_region_len(nor) ((nor)->params->otp.org->len)
#define spi_nor_otp_n_regions(nor) ((nor)->params->otp.org->n_regions)
+/**
+ * spi_nor_otp_read_secr() - read OTP data
+ * @nor: pointer to 'struct spi_nor'
+ * @from: offset to read from
+ * @len:number of bytes to read
+ * @buf:pointer to dst buffer
+ *
+ * Read OTP data from one region by using the SPINOR_OP_RSECR commands. This
+ * method is used on GigaDevice and Winbond flashes.
+ *
+ * Return: number of bytes read successfully, -errno otherwise
+ */
+int spi_nor_otp_read_secr(struct spi_nor *nor, loff_t addr, size_t len, u8
*buf)
+{
+ u8 addr_width, read_opcode, read_dummy;
+ struct spi_mem_dirmap_desc *rdesc;
+ enum spi_nor_protocol read_proto;
+ int ret;
+
+ read_opcode = nor->read_opcode;
+ addr_width = nor->addr_width;
+ read_dummy = nor->read_dummy;
+ read_proto = nor->read_proto;
+ rdesc = nor->dirmap.rdesc;
+
+ nor->read_opcode = SPINOR_OP_RSECR;
+ nor->addr_width = 3;
+ nor->read_dummy = 8;
+ nor->read_proto = SNOR_PROTO_1_1_1;
+ nor->dirmap.rdesc = NULL;
+
+ ret = spi_nor_read_data(nor, addr, len, buf);
+
+ nor->read_opcode = read_opcode;
+ nor->addr_width = addr_width;
+ nor->read_dummy = read_dummy;
+ nor->read_proto = read_proto;
+ nor->dirmap.rdesc = rdesc;
+
+ return ret;
+}
+
+/**
+ * spi_nor_otp_write_secr() - write OTP data
+ * @nor:pointer to 'struct spi_nor'
+ * @to: offset to write to
+ * @len:number of bytes to write
+ * @buf:pointer to src buffer
+ *
+ * Write OTP data to one region by using the SPINOR_OP_PSECR commands. This
+ * method is used on GigaDevice and Winbond flashes.
+ *
+ * Please note, the write must not span multiple OTP regions.
+ *
+ * Return: number of bytes written successfully, -errno otherwise
+ */
+int spi_nor_otp_write_secr(struct spi_nor *nor, loff_t addr, size_t len, u8
*buf)
+{
+ enum spi_nor_protocol write_proto;
+ struct spi_mem_dirmap_desc *wdesc;
+ u8 addr_width, program_opcode;
+ int ret, writt
[PATCH v5 3/3] mtd: spi-nor: winbond: add OTP support to w25q32fw/jw
With all the helper functions in place, add OTP support for the Winbond
W25Q32JW and W25Q32FW.
Both were tested on a LS1028A SoC with a NXP FSPI controller.
Signed-off-by: Michael Walle
---
drivers/mtd/spi-nor/winbond.c | 17 +++--
1 file changed, 15 insertions(+), 2 deletions(-)
diff --git a/drivers/mtd/spi-nor/winbond.c b/drivers/mtd/spi-nor/winbond.c
index 20b2c6f19d6f..9a81c67a60c6 100644
--- a/drivers/mtd/spi-nor/winbond.c
+++ b/drivers/mtd/spi-nor/winbond.c
@@ -54,14 +54,18 @@ static const struct flash_info winbond_parts[] = {
{ "w25q32", INFO(0xef4016, 0, 64 * 1024, 64, SECT_4K) },
{ "w25q32dw", INFO(0xef6016, 0, 64 * 1024, 64,
SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ |
- SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB) },
+ SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB)
+ OTP_INFO(256, 3, 0x1000, 0x1000)
+ },
+
{ "w25q32jv", INFO(0xef7016, 0, 64 * 1024, 64,
SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ |
SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB)
},
{ "w25q32jwm", INFO(0xef8016, 0, 64 * 1024, 64,
SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ |
- SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB) },
+ SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB)
+ OTP_INFO(256, 3, 0x1000, 0x1000) },
{ "w25q64jwm", INFO(0xef8017, 0, 64 * 1024, 128,
SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ |
SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB) },
@@ -132,9 +136,18 @@ static int winbond_set_4byte_addr_mode(struct spi_nor
*nor, bool enable)
return spi_nor_write_disable(nor);
}
+static const struct spi_nor_otp_ops winbond_otp_ops = {
+ .read = spi_nor_otp_read_secr,
+ .write = spi_nor_otp_write_secr,
+ .lock = spi_nor_otp_lock_sr2,
+ .is_locked = spi_nor_otp_is_locked_sr2,
+};
+
static void winbond_default_init(struct spi_nor *nor)
{
nor->params->set_4byte_addr_mode = winbond_set_4byte_addr_mode;
+ if (nor->params->otp.org->n_regions)
+ nor->params->otp.ops = _otp_ops;
}
static const struct spi_nor_fixups winbond_fixups = {
--
2.20.1
[PATCH v5 1/3] mtd: spi-nor: add OTP support
SPI flashes sometimes have a special OTP area, which can (and is) used to
store immutable properties like board serial number or vendor assigned
network hardware addresses.
The MTD subsystem already supports accessing such areas and some (non
SPI NOR) flashes already implement support for it. It differentiates
between user and factory areas. User areas can be written by the user and
factory ones are pre-programmed and locked down by the vendor, usually
containing an "electrical serial number". This patch will only add support
for the user areas.
Lay the foundation and implement the MTD callbacks for the SPI NOR and add
necessary parameters to the flash_info structure. If a flash supports OTP
it can be added by the convenience macro OTP_INFO(). Sometimes there are
individual regions, which might have individual offsets. Therefore, it is
possible to specify the starting address of the first regions as well as
the distance between two regions (e.g. Winbond devices uses this method).
Additionally, the regions might be locked down. Once locked, no further
write access is possible.
For SPI NOR flashes the OTP area is accessed like the normal memory, e.g.
by offset addressing; except that you either have to use special read/write
commands (Winbond) or you have to enter (and exit) a specific OTP mode
(Macronix, Micron).
Thus we introduce four operations to which the MTD callbacks will be
mapped: .read(), .write(), .lock() and .is_locked(). The read and the write
ops will be given an address offset to operate on while the locking ops use
regions because locking always affects a whole region. It is up to the
flash driver to implement these ops.
Signed-off-by: Michael Walle
---
drivers/mtd/spi-nor/Makefile | 2 +-
drivers/mtd/spi-nor/core.c | 5 +
drivers/mtd/spi-nor/core.h | 54 +
drivers/mtd/spi-nor/otp.c| 217 +++
4 files changed, 277 insertions(+), 1 deletion(-)
create mode 100644 drivers/mtd/spi-nor/otp.c
diff --git a/drivers/mtd/spi-nor/Makefile b/drivers/mtd/spi-nor/Makefile
index 653923896205..8a526554f2e2 100644
--- a/drivers/mtd/spi-nor/Makefile
+++ b/drivers/mtd/spi-nor/Makefile
@@ -1,6 +1,6 @@
# SPDX-License-Identifier: GPL-2.0
-spi-nor-objs := core.o sfdp.o
+spi-nor-objs := core.o sfdp.o otp.o
spi-nor-objs += atmel.o
spi-nor-objs += catalyst.o
spi-nor-objs += eon.o
diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index fbc34158a883..5e8218e53ec7 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -2918,6 +2918,8 @@ static void spi_nor_info_init_params(struct spi_nor *nor)
params->quad_enable = spi_nor_sr2_bit1_quad_enable;
params->set_4byte_addr_mode = spansion_set_4byte_addr_mode;
params->setup = spi_nor_default_setup;
+ nor->params->otp.org = >otp_org;
+
/* Default to 16-bit Write Status (01h) Command */
nor->flags |= SNOR_F_HAS_16BIT_SR;
@@ -3556,6 +3558,9 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
if (ret)
return ret;
+ /* Configure OTP parameters and ops */
+ spi_nor_otp_init(nor);
+
dev_info(dev, "%s (%lld Kbytes)\n", info->name,
(long long)mtd->size >> 10);
diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
index db07832ee66c..18521603b012 100644
--- a/drivers/mtd/spi-nor/core.h
+++ b/drivers/mtd/spi-nor/core.h
@@ -187,6 +187,45 @@ struct spi_nor_locking_ops {
int (*is_locked)(struct spi_nor *nor, loff_t ofs, uint64_t len);
};
+/**
+ * struct spi_nor_otp_organization - Structure to describe the SPI NOR OTP
regions
+ * @len: size of one OTP region in bytes.
+ * @base: start address of the OTP area.
+ * @offset:offset between consecutive OTP regions if there are more
+ * than one.
+ * @n_regions: number of individual OTP regions.
+ */
+struct spi_nor_otp_organization {
+ size_t len;
+ loff_t base;
+ loff_t offset;
+ unsigned int n_regions;
+};
+
+/**
+ * struct spi_nor_otp_ops - SPI NOR OTP methods
+ * @read: read from the SPI NOR OTP area.
+ * @write: write to the SPI NOR OTP area.
+ * @lock: lock an OTP region.
+ * @is_locked: check if an OTP region of the SPI NOR is locked.
+ */
+struct spi_nor_otp_ops {
+ int (*read)(struct spi_nor *nor, loff_t addr, size_t len, u8 *buf);
+ int (*write)(struct spi_nor *nor, loff_t addr, size_t len, u8 *buf);
+ int (*lock)(struct spi_nor *nor, unsigned int region);
+ int (*is_locked)(struct spi_nor *nor, unsigned int region);
+};
+
+/**
+ * struct spi_nor_otp - SPI NOR OTP grouping structure
+ * @org: OTP region organization
+ * @ops: OTP access ops
+ */
+struct spi_nor_otp {
+ const struct spi_nor_otp_organization *org;
+ con
[PATCH v5 0/3] mtd: spi-nor: OTP support
This patchset implements the MTD OTP functions to allow access to the SPI OTP data. Specific support is added for Winbond flash chips. In the past there was already an attempt by Rahul Bedarkar to add this, but there was no response. These patches are slightly based on his work. https://lore.kernel.org/linux-mtd/1489754636-21461-1-git-send-email-rahul.bedar...@imgtec.com/ Changes since v4: - s/u_char/u8/ - dropped extra whitespace - moved nor->params->otp.org assignment - moved spi_nor_otp_init() after spi_nor_init() - keep fixups as last property in struct spi_nor_flash_parameter - dropped spi_nor_otp_ops() - use i instead of region - move otp.o to the list of core objects Changes since v3: - remapped the OTP regions to a contiguous area starting at 0. The chips/cfi_cmdset_000[12].c remap the regions, too. - with that in place, read/write/lock/erase spanning multiple OTP regions are possible - picked up Tudors review remarks - added new erase support as RFC because MTD API/ABI is still missing. Feel free to review, but don't apply it. Changes since v2: - improved commit messages - add buffer size check in spi_nor_mtd_otp_info(). just to be sure, the buffer is hardcoded to 4k by the mtd subsys - moved all code to otp.c - dropped the patches introduced in v2 Changes since v1: - added methods for Macronix and similar flashes - added patch to cleanup/consolidate code in core.c Michael Walle (3): mtd: spi-nor: add OTP support mtd: spi-nor: implement OTP support for Winbond and similar flashes mtd: spi-nor: winbond: add OTP support to w25q32fw/jw drivers/mtd/spi-nor/Makefile | 2 +- drivers/mtd/spi-nor/core.c| 7 +- drivers/mtd/spi-nor/core.h| 60 ++ drivers/mtd/spi-nor/otp.c | 384 ++ drivers/mtd/spi-nor/winbond.c | 17 +- include/linux/mtd/spi-nor.h | 8 + 6 files changed, 474 insertions(+), 4 deletions(-) create mode 100644 drivers/mtd/spi-nor/otp.c -- 2.20.1
Re: [PATCH 2/2] mtd: spi-nor: add initial sysfs support
Hi Yicong,
Am 2021-03-20 05:16, schrieb Yicong Yang:
On 2021/3/18 17:24, Michael Walle wrote:
Add support to show the name and JEDEC identifier as well as to dump
the
SFDP table. Not all flashes list their SFDP table contents in their
datasheet. So having that is useful. It might also be helpful in bug
reports from users.
The idea behind the sysfs module is also to have raw access to the SPI
NOR flash device registers, which can also be useful for debugging.
I like the idea to dump the sfdp data,it will make debug easier.
should it go in debugfs?
we already have debugfs files for partname and partid of the flash.
I've seen that, but thats for the MTD subsystem and is per MTD. Well,
one could add an own debugfs for spi-nor, but I still fear it might
not be available just when its needed. Of course a developer can
easily enable it for its debugging kernel. But I'm not sure if thats
the only use case.
I'd guess it boils down to, whether there could also be tooling
around this. Eg. think of something like "lssfdp".
I'd prefer sysfs, but lets hear what the maintainers think.
[..]
+static ssize_t name_show(struct device *dev,
+struct device_attribute *attr, char *buf)
+{
+ struct spi_device *spi = to_spi_device(dev);
+ struct spi_mem *spimem = spi_get_drvdata(spi);
+ struct spi_nor *nor = spi_mem_get_drvdata(spimem);
+
+ return sprintf(buf, "%s\n", nor->info->name);
perhaps sysfs_emit() instead if we go sysfs? as suggested by [1].
Thanks, didn't know about that new helper.
-michael
[PATCH] arm64: configs: enable FlexTimer alarm timer
This driver is used on Layerscape SoCs to wake up the system from standby. It works in conjunction with the RCPM driver. The latter is only available as a builtin. Signed-off-by: Michael Walle --- arch/arm64/configs/defconfig | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig index 3f059bca9e24..58ce14b39637 100644 --- a/arch/arm64/configs/defconfig +++ b/arch/arm64/configs/defconfig @@ -884,6 +884,7 @@ CONFIG_RTC_DRV_DS3232=y CONFIG_RTC_DRV_PCF2127=m CONFIG_RTC_DRV_EFI=y CONFIG_RTC_DRV_CROS_EC=y +CONFIG_RTC_DRV_FSL_FTM_ALARM=m CONFIG_RTC_DRV_S3C=y CONFIG_RTC_DRV_PL031=y CONFIG_RTC_DRV_SUN6I=y @@ -997,6 +998,7 @@ CONFIG_OWL_PM_DOMAINS=y CONFIG_RASPBERRYPI_POWER=y CONFIG_FSL_DPAA=y CONFIG_FSL_MC_DPIO=y +CONFIG_FSL_RCPM=y CONFIG_QCOM_AOSS_QMP=y CONFIG_QCOM_COMMAND_DB=y CONFIG_QCOM_GENI_SE=y -- 2.20.1
Re: [PATCH v2] gpio: mpc8xxx: Add ACPI support
Am 2021-03-19 03:53, schrieb Ran Wang:
Current implementation only supports DT, now add ACPI support.
Note that compared to device of 'fsl,qoriq-gpio', LS1028A and
LS1088A's GPIO have no extra programming, so simplify related checking.
Signed-off-by: Ran Wang
---
Change in v2:
- Initialize devtype with NULL to fix compile warning.
- Replace of_device_get_match_data() and acpi_match_device with
device_get_match_data().
- Replace acpi_match_device() with simpler checking logic per Andy's
suggestion.
drivers/gpio/gpio-mpc8xxx.c | 34 +++---
1 file changed, 23 insertions(+), 11 deletions(-)
diff --git a/drivers/gpio/gpio-mpc8xxx.c b/drivers/gpio/gpio-mpc8xxx.c
index 6dfca83bcd90..646225fa3e73 100644
--- a/drivers/gpio/gpio-mpc8xxx.c
+++ b/drivers/gpio/gpio-mpc8xxx.c
@@ -9,6 +9,7 @@
* kind, whether express or implied.
*/
+#include
#include
#include
#include
@@ -292,8 +293,6 @@ static const struct of_device_id mpc8xxx_gpio_ids[]
= {
{ .compatible = "fsl,mpc5121-gpio", .data = _gpio_devtype, },
{ .compatible = "fsl,mpc5125-gpio", .data = _gpio_devtype, },
{ .compatible = "fsl,pq3-gpio", },
- { .compatible = "fsl,ls1028a-gpio", },
- { .compatible = "fsl,ls1088a-gpio", },
{ .compatible = "fsl,qoriq-gpio", },
{}
};
@@ -303,8 +302,8 @@ static int mpc8xxx_probe(struct platform_device
*pdev)
struct device_node *np = pdev->dev.of_node;
struct mpc8xxx_gpio_chip *mpc8xxx_gc;
struct gpio_chip*gc;
- const struct mpc8xxx_gpio_devtype *devtype =
- of_device_get_match_data(>dev);
+ const struct mpc8xxx_gpio_devtype *devtype = NULL;
+ struct fwnode_handle *fwnode;
int ret;
mpc8xxx_gc = devm_kzalloc(>dev, sizeof(*mpc8xxx_gc),
GFP_KERNEL);
@@ -315,14 +314,14 @@ static int mpc8xxx_probe(struct platform_device
*pdev)
raw_spin_lock_init(_gc->lock);
- mpc8xxx_gc->regs = of_iomap(np, 0);
+ mpc8xxx_gc->regs = devm_platform_ioremap_resource(pdev, 0);
if (!mpc8xxx_gc->regs)
return -ENOMEM;
gc = _gc->gc;
gc->parent = >dev;
- if (of_property_read_bool(np, "little-endian")) {
+ if (device_property_read_bool(>dev, "little-endian")) {
ret = bgpio_init(gc, >dev, 4,
mpc8xxx_gc->regs + GPIO_DAT,
NULL, NULL,
@@ -345,6 +344,7 @@ static int mpc8xxx_probe(struct platform_device
*pdev)
mpc8xxx_gc->direction_output = gc->direction_output;
+ devtype = device_get_match_data(>dev);
if (!devtype)
devtype = _gpio_devtype_default;
@@ -369,9 +369,9 @@ static int mpc8xxx_probe(struct platform_device
*pdev)
* associated input enable must be set (GPIOxGPIE[IEn]=1) to
propagate
* the port value to the GPIO Data Register.
*/
+ fwnode = dev_fwnode(>dev);
if (of_device_is_compatible(np, "fsl,qoriq-gpio") ||
- of_device_is_compatible(np, "fsl,ls1028a-gpio") ||
- of_device_is_compatible(np, "fsl,ls1088a-gpio"))
Again, please keep this. The DT bindings don't mention "fsl,qoriq-gpio"
is required. Alternatively, change the binding (ideally convert it to
YAML) and get an ack by Rob.
-michael
Re: [PATCH v2 net-next] net: phy: at803x: remove at803x_aneg_done()
Am 2021-03-18 20:44, schrieb Michael Walle: Here is what Vladimir says about it: at803x_aneg_done() keeps the aneg reporting as "not done" even when the copper-side link was reported as up, but the in-band autoneg has not finished. That was the _intended_ behavior when that code was introduced, and Heiner have said about it [1]: | That's not nice from the PHY: | It signals "link up", and if the system asks the PHY for link details, | then it sheepishly says "well, link is *almost* up". If the specification of phy_aneg_done behavior does not include in-band autoneg (and it doesn't), then this piece of code does not belong here. The fact that we can no longer trigger this code from phylib is yet another reason why it fails at its intended (and wrong) purpose and should be removed. Removing the SGMII link check, would just keep the call to genphy_aneg_done(), which is also the fallback. Thus we can just remove at803x_aneg_done() altogether. [1] https://lore.kernel.org/netdev/fdf0074a-2572-5914-6f3e-77202cbf9...@gmail.com/ Suggested-by: Vladimir Oltean Signed-off-by: Michael Walle --- Sorry forgot the version history: Changes since v1: - more detailed commit message -michael
[PATCH v2 net-next] net: phy: at803x: remove at803x_aneg_done()
Here is what Vladimir says about it:
at803x_aneg_done() keeps the aneg reporting as "not done" even when
the copper-side link was reported as up, but the in-band autoneg has
not finished.
That was the _intended_ behavior when that code was introduced, and
Heiner have said about it [1]:
| That's not nice from the PHY:
| It signals "link up", and if the system asks the PHY for link details,
| then it sheepishly says "well, link is *almost* up".
If the specification of phy_aneg_done behavior does not include
in-band autoneg (and it doesn't), then this piece of code does not
belong here.
The fact that we can no longer trigger this code from phylib is yet
another reason why it fails at its intended (and wrong) purpose and
should be removed.
Removing the SGMII link check, would just keep the call to
genphy_aneg_done(), which is also the fallback. Thus we can just remove
at803x_aneg_done() altogether.
[1]
https://lore.kernel.org/netdev/fdf0074a-2572-5914-6f3e-77202cbf9...@gmail.com/
Suggested-by: Vladimir Oltean
Signed-off-by: Michael Walle
---
drivers/net/phy/at803x.c | 31 ---
1 file changed, 31 deletions(-)
diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
index c2aa4c92edde..d7799beb811c 100644
--- a/drivers/net/phy/at803x.c
+++ b/drivers/net/phy/at803x.c
@@ -751,36 +751,6 @@ static void at803x_link_change_notify(struct phy_device
*phydev)
}
}
-static int at803x_aneg_done(struct phy_device *phydev)
-{
- int ccr;
-
- int aneg_done = genphy_aneg_done(phydev);
- if (aneg_done != BMSR_ANEGCOMPLETE)
- return aneg_done;
-
- /*
-* in SGMII mode, if copper side autoneg is successful,
-* also check SGMII side autoneg result
-*/
- ccr = phy_read(phydev, AT803X_REG_CHIP_CONFIG);
- if ((ccr & AT803X_MODE_CFG_MASK) != AT803X_MODE_CFG_SGMII)
- return aneg_done;
-
- /* switch to SGMII/fiber page */
- phy_write(phydev, AT803X_REG_CHIP_CONFIG, ccr & ~AT803X_BT_BX_REG_SEL);
-
- /* check if the SGMII link is OK. */
- if (!(phy_read(phydev, AT803X_PSSR) & AT803X_PSSR_MR_AN_COMPLETE)) {
- phydev_warn(phydev, "803x_aneg_done: SGMII link is not ok\n");
- aneg_done = 0;
- }
- /* switch back to copper page */
- phy_write(phydev, AT803X_REG_CHIP_CONFIG, ccr | AT803X_BT_BX_REG_SEL);
-
- return aneg_done;
-}
-
static int at803x_read_status(struct phy_device *phydev)
{
int ss, err, old_link = phydev->link;
@@ -1198,7 +1168,6 @@ static struct phy_driver at803x_driver[] = {
.resume = at803x_resume,
/* PHY_GBIT_FEATURES */
.read_status= at803x_read_status,
- .aneg_done = at803x_aneg_done,
.config_intr= _config_intr,
.handle_interrupt = at803x_handle_interrupt,
.get_tunable= at803x_get_tunable,
--
2.20.1
[PATCH 1/2] arm64: dts: fsl-ls1028a-kontron-sl28: move MTD partitions
Move the MTD partitions to the partitions subnode. This is the new way
to specify the partitions, see
Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml
Signed-off-by: Michael Walle
---
.../freescale/fsl-ls1028a-kontron-sl28.dts| 94 ++-
1 file changed, 49 insertions(+), 45 deletions(-)
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1028a-kontron-sl28.dts
b/arch/arm64/boot/dts/freescale/fsl-ls1028a-kontron-sl28.dts
index 0516076087ae..7e3a33eb2045 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1028a-kontron-sl28.dts
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a-kontron-sl28.dts
@@ -115,8 +115,6 @@
status = "okay";
flash@0 {
- #address-cells = <1>;
- #size-cells = <1>;
compatible = "jedec,spi-nor";
m25p,fast-read;
spi-max-frequency = <13300>;
@@ -125,49 +123,55 @@
spi-rx-bus-width = <2>; /* 2 SPI Rx lines */
spi-tx-bus-width = <1>; /* 1 SPI Tx line */
- partition@0 {
- reg = <0x00 0x01>;
- label = "rcw";
- read-only;
- };
-
- partition@1 {
- reg = <0x01 0x0f>;
- label = "failsafe bootloader";
- read-only;
- };
-
- partition@10 {
- reg = <0x10 0x04>;
- label = "failsafe DP firmware";
- read-only;
- };
-
- partition@14 {
- reg = <0x14 0x0a>;
- label = "failsafe trusted firmware";
- read-only;
- };
-
- partition@1e {
- reg = <0x1e 0x02>;
- label = "reserved";
- read-only;
- };
-
- partition@20 {
- reg = <0x20 0x01>;
- label = "configuration store";
- };
-
- partition@21 {
- reg = <0x21 0x1d>;
- label = "bootloader";
- };
-
- partition@3e {
- reg = <0x3e 0x02>;
- label = "bootloader environment";
+ partitions {
+ compatible = "fixed-partitions";
+ #address-cells = <1>;
+ #size-cells = <1>;
+
+ partition@0 {
+ reg = <0x00 0x01>;
+ label = "rcw";
+ read-only;
+ };
+
+ partition@1 {
+ reg = <0x01 0x0f>;
+ label = "failsafe bootloader";
+ read-only;
+ };
+
+ partition@10 {
+ reg = <0x10 0x04>;
+ label = "failsafe DP firmware";
+ read-only;
+ };
+
+ partition@14 {
+ reg = <0x14 0x0a>;
+ label = "failsafe trusted firmware";
+ read-only;
+ };
+
+ partition@1e {
+ reg = <0x1e 0x02>;
+ label = "reserved";
+ read-only;
+ };
+
+ partition@20 {
+ reg = <0x20 0x01>;
+ label = "configuration store";
+ };
+
+ partition@21 {
+ reg = <0x21 0x1d>;
+ label = "bootloader";
+ };
+
+ partition@3e {
+ reg = <0x3e 0x02>;
+ label = "bootloader environment";
+ };
};
};
};
--
2.20.1
[PATCH 2/2] arm64: dts: fsl-ls1028a-kontron-sl28: combine unused partitions
The failsafe partitions for the DP firmware and for AT-F are unused. If
AT-F will ever be supported in the failsafe mode, then it will be a FIT
image. Thus fold the unused partitions into the failsafe bootloader one
to have enough storage if the bootloader image will grow.
While at it, remove the reserved partition. It served no purpose other
than having no hole in the map.
Signed-off-by: Michael Walle
---
.../freescale/fsl-ls1028a-kontron-sl28.dts| 20 +--
1 file changed, 1 insertion(+), 19 deletions(-)
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1028a-kontron-sl28.dts
b/arch/arm64/boot/dts/freescale/fsl-ls1028a-kontron-sl28.dts
index 7e3a33eb2045..311e3aae0a3c 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1028a-kontron-sl28.dts
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a-kontron-sl28.dts
@@ -135,29 +135,11 @@
};
partition@1 {
- reg = <0x01 0x0f>;
+ reg = <0x01 0x1d>;
label = "failsafe bootloader";
read-only;
};
- partition@10 {
- reg = <0x10 0x04>;
- label = "failsafe DP firmware";
- read-only;
- };
-
- partition@14 {
- reg = <0x14 0x0a>;
- label = "failsafe trusted firmware";
- read-only;
- };
-
- partition@1e {
- reg = <0x1e 0x02>;
- label = "reserved";
- read-only;
- };
-
partition@20 {
reg = <0x20 0x01>;
label = "configuration store";
--
2.20.1
Re: [PATCH net-next] net: phy: at803x: remove at803x_aneg_done()
Am 2021-03-18 17:21, schrieb Heiner Kallweit: On 18.03.2021 16:17, Vladimir Oltean wrote: On Thu, Mar 18, 2021 at 03:54:00PM +0100, Heiner Kallweit wrote: On 18.03.2021 15:23, Michael Walle wrote: at803x_aneg_done() is pretty much dead code since the patch series "net: phy: improve and simplify phylib state machine" [1]. Remove it. Well, it's not dead, it's resting .. There are few places where phy_aneg_done() is used. So you would need to explain: - why these users can't be used with this PHY driver - or why the aneg_done callback isn't needed here and the genphy_aneg_done() fallback is sufficient The piece of code that Michael is removing keeps the aneg reporting as "not done" even when the copper-side link was reported as up, but the in-band autoneg has not finished. That was the _intended_ behavior when that code was introduced, and you have said about it: https://www.spinics.net/lists/stable/msg389193.html | That's not nice from the PHY: | It signals "link up", and if the system asks the PHY for link details, | then it sheepishly says "well, link is *almost* up". If the specification of phy_aneg_done behavior does not include in-band autoneg (and it doesn't), then this piece of code does not belong here. The fact that we can no longer trigger this code from phylib is yet another reason why it fails at its intended (and wrong) purpose and should be removed. I don't argue against the change, I just think that the current commit description isn't sufficient. What you just said I would have expected in the commit description. I'll come up with a better one, Vladimir, may I use parts of the text above? -michael
[PATCH net-next] net: phy: at803x: remove at803x_aneg_done()
at803x_aneg_done() is pretty much dead code since the patch series
"net: phy: improve and simplify phylib state machine" [1]. Remove it.
[1]
https://lore.kernel.org/netdev/922c223b-7bc0-e0ec-345d-2034b796a...@gmail.com/
Suggested-by: Vladimir Oltean
Signed-off-by: Michael Walle
---
drivers/net/phy/at803x.c | 31 ---
1 file changed, 31 deletions(-)
diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
index c2aa4c92edde..d7799beb811c 100644
--- a/drivers/net/phy/at803x.c
+++ b/drivers/net/phy/at803x.c
@@ -751,36 +751,6 @@ static void at803x_link_change_notify(struct phy_device
*phydev)
}
}
-static int at803x_aneg_done(struct phy_device *phydev)
-{
- int ccr;
-
- int aneg_done = genphy_aneg_done(phydev);
- if (aneg_done != BMSR_ANEGCOMPLETE)
- return aneg_done;
-
- /*
-* in SGMII mode, if copper side autoneg is successful,
-* also check SGMII side autoneg result
-*/
- ccr = phy_read(phydev, AT803X_REG_CHIP_CONFIG);
- if ((ccr & AT803X_MODE_CFG_MASK) != AT803X_MODE_CFG_SGMII)
- return aneg_done;
-
- /* switch to SGMII/fiber page */
- phy_write(phydev, AT803X_REG_CHIP_CONFIG, ccr & ~AT803X_BT_BX_REG_SEL);
-
- /* check if the SGMII link is OK. */
- if (!(phy_read(phydev, AT803X_PSSR) & AT803X_PSSR_MR_AN_COMPLETE)) {
- phydev_warn(phydev, "803x_aneg_done: SGMII link is not ok\n");
- aneg_done = 0;
- }
- /* switch back to copper page */
- phy_write(phydev, AT803X_REG_CHIP_CONFIG, ccr | AT803X_BT_BX_REG_SEL);
-
- return aneg_done;
-}
-
static int at803x_read_status(struct phy_device *phydev)
{
int ss, err, old_link = phydev->link;
@@ -1198,7 +1168,6 @@ static struct phy_driver at803x_driver[] = {
.resume = at803x_resume,
/* PHY_GBIT_FEATURES */
.read_status= at803x_read_status,
- .aneg_done = at803x_aneg_done,
.config_intr= _config_intr,
.handle_interrupt = at803x_handle_interrupt,
.get_tunable= at803x_get_tunable,
--
2.20.1
[PATCH 2/2] mtd: spi-nor: add initial sysfs support
Add support to show the name and JEDEC identifier as well as to dump the
SFDP table. Not all flashes list their SFDP table contents in their
datasheet. So having that is useful. It might also be helpful in bug
reports from users.
The idea behind the sysfs module is also to have raw access to the SPI
NOR flash device registers, which can also be useful for debugging.
Signed-off-by: Michael Walle
---
drivers/mtd/spi-nor/Makefile | 2 +-
drivers/mtd/spi-nor/core.c | 5 +++
drivers/mtd/spi-nor/core.h | 3 ++
drivers/mtd/spi-nor/sysfs.c | 86
4 files changed, 95 insertions(+), 1 deletion(-)
create mode 100644 drivers/mtd/spi-nor/sysfs.c
diff --git a/drivers/mtd/spi-nor/Makefile b/drivers/mtd/spi-nor/Makefile
index 653923896205..aff308f75987 100644
--- a/drivers/mtd/spi-nor/Makefile
+++ b/drivers/mtd/spi-nor/Makefile
@@ -1,6 +1,6 @@
# SPDX-License-Identifier: GPL-2.0
-spi-nor-objs := core.o sfdp.o
+spi-nor-objs := core.o sfdp.o sysfs.o
spi-nor-objs += atmel.o
spi-nor-objs += catalyst.o
spi-nor-objs += eon.o
diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index 4a315cb1c4db..2eaf4ba8c0f3 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -3707,6 +3707,10 @@ static int spi_nor_probe(struct spi_mem *spimem)
if (ret)
return ret;
+ ret = spi_nor_sysfs_create(nor);
+ if (ret)
+ return ret;
+
return mtd_device_register(>mtd, data ? data->parts : NULL,
data ? data->nr_parts : 0);
}
@@ -3716,6 +3720,7 @@ static int spi_nor_remove(struct spi_mem *spimem)
struct spi_nor *nor = spi_mem_get_drvdata(spimem);
spi_nor_restore(nor);
+ spi_nor_sysfs_remove(nor);
/* Clean up MTD stuff. */
return mtd_device_unregister(>mtd);
diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
index 668f22011b1d..dd592f7b62d1 100644
--- a/drivers/mtd/spi-nor/core.h
+++ b/drivers/mtd/spi-nor/core.h
@@ -488,4 +488,7 @@ static struct spi_nor __maybe_unused *mtd_to_spi_nor(struct
mtd_info *mtd)
return mtd->priv;
}
+int spi_nor_sysfs_create(struct spi_nor *nor);
+void spi_nor_sysfs_remove(struct spi_nor *nor);
+
#endif /* __LINUX_MTD_SPI_NOR_INTERNAL_H */
diff --git a/drivers/mtd/spi-nor/sysfs.c b/drivers/mtd/spi-nor/sysfs.c
new file mode 100644
index ..0de031e246c5
--- /dev/null
+++ b/drivers/mtd/spi-nor/sysfs.c
@@ -0,0 +1,86 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include
+#include
+#include
+#include
+
+#include "core.h"
+
+static ssize_t name_show(struct device *dev,
+struct device_attribute *attr, char *buf)
+{
+ struct spi_device *spi = to_spi_device(dev);
+ struct spi_mem *spimem = spi_get_drvdata(spi);
+ struct spi_nor *nor = spi_mem_get_drvdata(spimem);
+
+ return sprintf(buf, "%s\n", nor->info->name);
+}
+static DEVICE_ATTR_RO(name);
+
+static ssize_t jedec_id_show(struct device *dev,
+struct device_attribute *attr, char *buf)
+{
+ struct spi_device *spi = to_spi_device(dev);
+ struct spi_mem *spimem = spi_get_drvdata(spi);
+ struct spi_nor *nor = spi_mem_get_drvdata(spimem);
+
+ return sprintf(buf, "%*phN\n", nor->info->id_len, nor->info->id);
+}
+static DEVICE_ATTR_RO(jedec_id);
+
+static struct attribute *spi_nor_sysfs_entries[] = {
+ _attr_name.attr,
+ _attr_jedec_id.attr,
+ NULL
+};
+
+static ssize_t sfdp_read(struct file *filp, struct kobject *kobj,
+struct bin_attribute *bin_attr, char *buf,
+loff_t off, size_t count)
+{
+ struct spi_device *spi = to_spi_device(kobj_to_dev(kobj));
+ struct spi_mem *spimem = spi_get_drvdata(spi);
+ struct spi_nor *nor = spi_mem_get_drvdata(spimem);
+ struct sfdp *sfdp = nor->sfdp;
+ size_t sfdp_size = sfdp->num_dwords * sizeof(*sfdp->dwords);
+
+ return memory_read_from_buffer(buf, count, , nor->sfdp->dwords,
+ sfdp_size);
+}
+static BIN_ATTR_RO(sfdp, PAGE_SIZE);
+
+static struct bin_attribute *spi_nor_sysfs_bin_entries[] = {
+ _attr_sfdp,
+ NULL
+};
+
+static umode_t spi_nor_sysfs_is_bin_visible(struct kobject *kobj,
+ struct bin_attribute *attr, int n)
+{
+ struct spi_device *spi = to_spi_device(kobj_to_dev(kobj));
+ struct spi_mem *spimem = spi_get_drvdata(spi);
+ struct spi_nor *nor = spi_mem_get_drvdata(spimem);
+
+ if (attr == _attr_sfdp && nor->sfdp)
+ return 0444;
+
+ return 0;
+}
+
+static struct attribute_group spi_nor_sysfs_attr_group = {
+ .name = NULL,
+ .is_bi
[PATCH 1/2] mtd: spi-nor: sfdp: save a copy of the SFDP data
Due to possible mode switching to 8D-8D-8D, it might not be possible to
read the SFDP after the initial probe. To be able to dump the SFDP via
sysfs afterwards, make a complete copy of it.
Signed-off-by: Michael Walle
---
drivers/mtd/spi-nor/core.h | 10
drivers/mtd/spi-nor/sfdp.c | 49 +
include/linux/mtd/spi-nor.h | 3 +++
3 files changed, 62 insertions(+)
diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
index 4a3f7f150b5d..668f22011b1d 100644
--- a/drivers/mtd/spi-nor/core.h
+++ b/drivers/mtd/spi-nor/core.h
@@ -407,6 +407,16 @@ struct spi_nor_manufacturer {
const struct spi_nor_fixups *fixups;
};
+/**
+ * struct sfdp - SFDP data
+ * @num_dwords: number of entries in the dwords array
+ * @dwords: array of double words of the SFDP data
+ */
+struct sfdp {
+ size_t num_dwords;
+ u32 *dwords;
+};
+
/* Manufacturer drivers. */
extern const struct spi_nor_manufacturer spi_nor_atmel;
extern const struct spi_nor_manufacturer spi_nor_catalyst;
diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c
index 25142ec4737b..2b6c96e02532 100644
--- a/drivers/mtd/spi-nor/sfdp.c
+++ b/drivers/mtd/spi-nor/sfdp.c
@@ -16,6 +16,7 @@
(((p)->parameter_table_pointer[2] << 16) | \
((p)->parameter_table_pointer[1] << 8) | \
((p)->parameter_table_pointer[0] << 0))
+#define SFDP_PARAM_HEADER_PARAM_LEN(p) ((p)->length * 4)
#define SFDP_BFPT_ID 0xff00 /* Basic Flash Parameter Table */
#define SFDP_SECTOR_MAP_ID 0xff81 /* Sector Map Table */
@@ -1263,6 +1264,8 @@ int spi_nor_parse_sfdp(struct spi_nor *nor,
struct sfdp_parameter_header *param_headers = NULL;
struct sfdp_header header;
struct device *dev = nor->dev;
+ struct sfdp *sfdp;
+ size_t sfdp_size;
size_t psize;
int i, err;
@@ -1285,6 +1288,9 @@ int spi_nor_parse_sfdp(struct spi_nor *nor,
bfpt_header->major != SFDP_JESD216_MAJOR)
return -EINVAL;
+ sfdp_size = SFDP_PARAM_HEADER_PTP(bfpt_header) +
+ SFDP_PARAM_HEADER_PARAM_LEN(bfpt_header);
+
/*
* Allocate memory then read all parameter headers with a single
* Read SFDP command. These parameter headers will actually be parsed
@@ -1311,6 +1317,49 @@ int spi_nor_parse_sfdp(struct spi_nor *nor,
}
}
+ /*
+* Cache the complete SFDP data. It is not (easily) possible to fetch
+* SFDP after probe time and we need it for the sysfs access.
+*/
+ for (i = 0; i < header.nph; i++) {
+ param_header = _headers[i];
+ sfdp_size = max_t(size_t, sfdp_size,
+ SFDP_PARAM_HEADER_PTP(param_header) +
+ SFDP_PARAM_HEADER_PARAM_LEN(param_header));
+ }
+
+ /*
+* Limit the total size to a reasonable value to avoid allocating too
+* much memory just of because the flash returned some insane values.
+*/
+ if (sfdp_size > PAGE_SIZE) {
+ dev_dbg(dev, "SFDP data (%zu) too big, truncating\n",
+ sfdp_size);
+ sfdp_size = PAGE_SIZE;
+ }
+
+ sfdp = devm_kzalloc(dev, sizeof(*sfdp), GFP_KERNEL);
+ if (!sfdp) {
+ err = -ENOMEM;
+ goto exit;
+ }
+
+ sfdp->num_dwords = DIV_ROUND_UP(sfdp_size, sizeof(*sfdp->dwords));
+ sfdp->dwords = devm_kcalloc(dev, sfdp->num_dwords,
+ sizeof(*sfdp->dwords), GFP_KERNEL);
+ if (!sfdp->dwords) {
+ err = -ENOMEM;
+ goto exit;
+ }
+
+ err = spi_nor_read_sfdp_dma_unsafe(nor, 0, sfdp_size, sfdp->dwords);
+ if (err < 0) {
+ dev_dbg(dev, "failed to read SFDP data\n");
+ goto exit;
+ }
+
+ nor->sfdp = sfdp;
+
/*
* Check other parameter headers to get the latest revision of
* the basic flash parameter table.
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
index a0d572855444..55c550208949 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -351,6 +351,7 @@ enum spi_nor_cmd_ext {
struct flash_info;
struct spi_nor_manufacturer;
struct spi_nor_flash_parameter;
+struct sfdp;
/**
* struct spi_nor - Structure for defining the SPI NOR layer
@@ -375,6 +376,7 @@ struct spi_nor_flash_parameter;
* @read_proto:the SPI protocol for read operations
* @write_proto: the SPI protocol for write operations
* @reg_proto: the SPI protocol for read_reg/write_reg/erase operations
+ * @sfdp: the SFDP data of the flash
* @controller_ops:SPI NOR controller driver specific operations.
* @params:[FLASH-SPECIFIC] SPI
[PATCH 0/2] mtd: spi-nor: support dumping sfdp tables
Add the possibility to dump the SFDP data of a flash device. More and more flash devices share the same flash ID and we need per device fixups. Usually, these fixups differentiate flashes by looking at differences in the SFDP data. Determining the difference is only possible if we have the SFDP data for all the flashes which share a flash ID. This will lay the foundation to dump the whole SFDP data of a flash device. This is even more important, because some datasheets doesn't even contain the SFDP data. Fixups for these kind of flashes are nearly impossible to do. I envision having a database of all the SFDP data for the flashes we support and make it a requirement to submit it when a new flash is added. This might or might not have legal implications. Thus I'd start with having that database private to the SPI NOR maintainers. Changes since RFC: - Don't read SFDP data after probe. The flash might already be switched to 8D-8D-8D mode. Instead, cache the SFDP data - add two sysfs files: jedec-id and name - change the file mode of the sfdp file from 0400 to 0444. There is no hardware access anymore. Michael Walle (2): mtd: spi-nor: sfdp: save a copy of the SFDP data mtd: spi-nor: add initial sysfs support drivers/mtd/spi-nor/Makefile | 2 +- drivers/mtd/spi-nor/core.c | 5 +++ drivers/mtd/spi-nor/core.h | 13 ++ drivers/mtd/spi-nor/sfdp.c | 49 drivers/mtd/spi-nor/sysfs.c | 86 include/linux/mtd/spi-nor.h | 3 ++ 6 files changed, 157 insertions(+), 1 deletion(-) create mode 100644 drivers/mtd/spi-nor/sysfs.c -- 2.20.1
Re: [PATCH] arm64: dts: ls1028a: fix optee node
Am 2021-03-18 09:34, schrieb Michael Walle:
Don't enable the optee node in the SoC include. It is an optional
component and actually, if enabled, breaks boards which doesn't have
it.
This reverts commit 48787485f8de ("arm64: dts: ls1028a: enable optee
node") and enables the node per board, assuming the intend of the
original author was to enable OPTEE for the LS1028A-RDB and the
LS1028A-QDS.
Fixes: 48787485f8de ("arm64: dts: ls1028a: enable optee node")
Reported-by: Guillaume Tucker
Reported-by: "kernelci.org bot"
Tested-by: Michael Walle
Signed-off-by: Michael Walle
---
Please disregard this patch, because the offending patch was
already dropped:
https://lore.kernel.org/lkml/20210318084029.GY11246@dragon/
Sahil, if you like you can pick it up to enable the nodes for
your ls1028a boards.
-michael
[PATCH] arm64: dts: ls1028a: fix optee node
Don't enable the optee node in the SoC include. It is an optional
component and actually, if enabled, breaks boards which doesn't have it.
This reverts commit 48787485f8de ("arm64: dts: ls1028a: enable optee
node") and enables the node per board, assuming the intend of the
original author was to enable OPTEE for the LS1028A-RDB and the
LS1028A-QDS.
Fixes: 48787485f8de ("arm64: dts: ls1028a: enable optee node")
Reported-by: Guillaume Tucker
Reported-by: "kernelci.org bot"
Tested-by: Michael Walle
Signed-off-by: Michael Walle
---
arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dts | 4
arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts | 4
arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi| 3 ++-
3 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dts
b/arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dts
index fbcba9cb8503..060d3c79244d 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dts
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dts
@@ -327,6 +327,10 @@
status = "okay";
};
+ {
+ status = "okay";
+};
+
{
status = "okay";
};
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts
b/arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts
index 41ae6e7675ba..1bdf0104d492 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts
@@ -274,6 +274,10 @@
status = "okay";
};
+ {
+ status = "okay";
+};
+
{
status = "okay";
};
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
b/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
index 50d277eb2a54..e2007ebacd69 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
@@ -92,9 +92,10 @@
};
firmware {
- optee {
+ optee: optee {
compatible = "linaro,optee-tz";
method = "smc";
+ status = "disabled";
};
};
--
2.20.1
Re: [PATCH v2 4/5] mtd: spi-nor: Move Software Write Protection logic out of the core
Am 2021-03-17 10:30, schrieb tudor.amba...@microchip.com: soft-wr-protect.c or software-write-protect.c ? Having in mind that we have the SWP configs, I think I prefer swp.c. But let's see what majority thinks, we'll do as majority prefers. Michael, Pratyush? It's just an internal name, thus as long as it remotely makes sense, I'm fine. It's just a matter of taste, isn't it? Sure, it's a matter of preference. What's yours? if i have to choose, swp.c But here's one technical reason that would bother me more: name clashes between the core modules: core, sfdp, otp, swp and the vendor names. It is very unlikely, but there is a non-zero chance ;) We can move all manufacturers to a manufacturers/ folder. Each manufacturer driver will have to #include "../core.h", about what I have some mixed feelings. I don't think it makes sense as long as there is no clash; or until there are many more core modules, so you can't keep them apart anymore. It will just make it harder to follow the git history. -michael
Re: [PATCH v2 4/5] mtd: spi-nor: Move Software Write Protection logic out of the core
Am 2021-03-17 07:09, schrieb tudor.amba...@microchip.com: On 3/15/21 8:23 AM, Vignesh Raghavendra wrote: EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe On 3/9/21 12:58 PM, tudor.amba...@microchip.com wrote: On 3/8/21 7:28 PM, Vignesh Raghavendra wrote: EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe On 3/6/21 3:20 PM, Tudor Ambarus wrote: It makes the core file a bit smaller and provides better separation between the Software Write Protection features and the core logic. All the next generic software write protection features (e.g. Individual Block Protection) will reside in swp.c. Signed-off-by: Tudor Ambarus --- drivers/mtd/spi-nor/Makefile | 2 +- drivers/mtd/spi-nor/core.c | 407 +- drivers/mtd/spi-nor/core.h | 4 + drivers/mtd/spi-nor/swp.c| 419 +++ Hmmm, name swp.c does not seem intuitive to me. How about expanding it a bit: soft-wr-protect.c or software-write-protect.c ? Having in mind that we have the SWP configs, I think I prefer swp.c. But let's see what majority thinks, we'll do as majority prefers. Michael, Pratyush? It's just an internal name, thus as long as it remotely makes sense, I'm fine. It's just a matter of taste, isn't it? But here's one technical reason that would bother me more: name clashes between the core modules: core, sfdp, otp, swp and the vendor names. It is very unlikely, but there is a non-zero chance ;) -michael
Re: next/master bisection: baseline.login on kontron-kbox-a-230-ls
Am 2021-03-16 19:33, schrieb Guillaume Tucker:
Hi Sahil,
Please see the bisection report below about a boot failure on
kontron-kbox-a-230-ls on linux-next.
Reports aren't automatically sent to the public while we're
trialing new bisection features on kernelci.org but this one
looks valid.
nice! Thanks.
[..]
commit 48787485f8de44915016d4583e898b62bb2d5753
Author: Sahil Malhotra
Date: Fri Mar 5 14:03:51 2021 +0530
arm64: dts: ls1028a: enable optee node
optee node was disabled in ls1028a.dtsi, enabling it by default.
Please enable this per board. As it is also indicated by my original
commit f90931aeefe3 ("arm64: dts: ls1028a: add optee node") message:
Add the optee node which can either be enabled by a specific board or
by
the bootloader.
-michael
Re: [PATCH] dt-bindings: spi: Convert Freescale DSPI to json schema
Am 2021-03-16 19:06, schrieb Pratyush Yadav: On 16/03/21 06:45PM, Michael Walle wrote: Am 2021-03-15 19:30, schrieb Pratyush Yadav: .. > > +patternProperties: > > + "@[0-9a-f]+": Shouldn't this be "^.*@[0-9a-f]+$"? The pattern has to match _anywhere_ in the string so both should match the flash node. Your pattern is more "strict" or "precise". See the note at [0]. I know, but specifying the whole line is widely used in the bindings. -michael
Re: [PATCH] dt-bindings: spi: Convert Freescale DSPI to json schema
Am 2021-03-15 19:30, schrieb Pratyush Yadav: .. +patternProperties: + "@[0-9a-f]+": Shouldn't this be "^.*@[0-9a-f]+$"? +type: object + +properties: + fsl,spi-cs-sck-delay: +description: + Delay in nanoseconds between activating chip select and the start of + clock signal, at the start of a transfer. +$ref: /schemas/types.yaml#/definitions/uint32 + + fsl,spi-sck-cs-delay: +description: + Delay in nanoseconds between stopping the clock signal and + deactivating chip select, at the end of a transfer. +$ref: /schemas/types.yaml#/definitions/uint32 + .. -michael
Re: [RFC PATCH 1/3] mtd: spi-nor: sfdp: remember sfdp_size
Am 2021-03-16 12:06, schrieb Pratyush Yadav:
On 16/03/21 12:01PM, Michael Walle wrote:
Hi Pratyush,
Am 2021-03-16 11:42, schrieb Pratyush Yadav:
> On 12/03/21 08:05PM, Michael Walle wrote:
> > Save the sftp_size in the spi_nor struct so we can use it to dump the
> > SFDP table without parsing the headers again.
> >
> > Signed-off-by: Michael Walle
> > ---
> > drivers/mtd/spi-nor/sfdp.c | 12
> > include/linux/mtd/spi-nor.h | 1 +
> > 2 files changed, 13 insertions(+)
> >
> > diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c
> > index 25142ec4737b..b1814afefc33 100644
> > --- a/drivers/mtd/spi-nor/sfdp.c
> > +++ b/drivers/mtd/spi-nor/sfdp.c
> > @@ -16,6 +16,7 @@
> > (((p)->parameter_table_pointer[2] << 16) | \
> >((p)->parameter_table_pointer[1] << 8) | \
> >((p)->parameter_table_pointer[0] << 0))
> > +#define SFDP_PARAM_HEADER_PARAM_LEN(p) ((p)->length * 4)
> >
> > #define SFDP_BFPT_ID 0xff00 /* Basic Flash Parameter Table */
> > #define SFDP_SECTOR_MAP_ID 0xff81 /* Sector Map Table */
> > @@ -1263,6 +1264,7 @@ int spi_nor_parse_sfdp(struct spi_nor *nor,
> > struct sfdp_parameter_header *param_headers = NULL;
> > struct sfdp_header header;
> > struct device *dev = nor->dev;
> > + size_t param_max_offset;
> > size_t psize;
> > int i, err;
> >
> > @@ -1285,6 +1287,9 @@ int spi_nor_parse_sfdp(struct spi_nor *nor,
> > bfpt_header->major != SFDP_JESD216_MAJOR)
> > return -EINVAL;
> >
> > + nor->sfdp_size = SFDP_PARAM_HEADER_PTP(bfpt_header) +
> > + SFDP_PARAM_HEADER_PARAM_LEN(bfpt_header);
> > +
> > /*
> >* Allocate memory then read all parameter headers with a single
> >* Read SFDP command. These parameter headers will actually be
> > parsed
> > @@ -1311,6 +1316,13 @@ int spi_nor_parse_sfdp(struct spi_nor *nor,
> > }
> > }
> >
> > + for (i = 0; i < header.nph; i++) {
> > + param_header = _headers[i];
> > + param_max_offset = SFDP_PARAM_HEADER_PTP(param_header) +
> > +SFDP_PARAM_HEADER_PARAM_LEN(param_header);
> > + nor->sfdp_size = max(nor->sfdp_size, param_max_offset);
> > + }
> > +
>
> I don't see any mention in the SFDP spec (JESD216D-01) that parameter
> tables have to be contiguous.
ch. 6.1, fig.10 looks like all the headers come after the initial SFDP
header. If that wasn't the case, how would you find the headers then?
Also ch. 6.3
"""All subsequent parameter headers need to be contiguous and may be
specified..."""
> In fact, it says that "Parameter tables
> may be located anywhere in the SFDP space. They do not need to
> immediately follow the parameter headers".
The tables itself, yes, but not the headers for the tables.
Yes, I was talking about the tables and not the headers. There might be
holes in the SFDP space but I don't think it would be a problem.
Ah, ok. Well basically I'm assuming that the holes will returning some
sane values. Or you could dump the space together with a mask. But that
is overkill ;)
-michael
Re: [RFC PATCH 2/3] mtd: spi-nor: sfdp: fix spi_nor_read_sfdp()
Am 2021-03-16 12:04, schrieb Pratyush Yadav:
On 12/03/21 08:05PM, Michael Walle wrote:
If spi_nor_read_sfdp() is used after probe, we have to set read_proto
and the read dirmap.
Signed-off-by: Michael Walle
---
drivers/mtd/spi-nor/sfdp.c | 8
1 file changed, 8 insertions(+)
diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c
index b1814afefc33..47634ec9b899 100644
--- a/drivers/mtd/spi-nor/sfdp.c
+++ b/drivers/mtd/spi-nor/sfdp.c
@@ -179,19 +179,27 @@ static int spi_nor_read_sfdp(struct spi_nor
*nor, u32 addr,
size_t len, void *buf)
{
u8 addr_width, read_opcode, read_dummy;
+ struct spi_mem_dirmap_desc *rdesc;
+ enum spi_nor_protocol read_proto;
int ret;
read_opcode = nor->read_opcode;
+ read_proto = nor->read_proto;
+ rdesc = nor->dirmap.rdesc;
addr_width = nor->addr_width;
read_dummy = nor->read_dummy;
nor->read_opcode = SPINOR_OP_RDSFDP;
+ nor->read_proto = SNOR_PROTO_1_1_1;
+ nor->dirmap.rdesc = NULL;
nor->addr_width = 3;
nor->read_dummy = 8;
NACK. You can't assume the device is still in 1S-1S-1S mode after
probe.
For example, the s28hs512t flash is switched to 8D-8D-8D mode by the
time the probe finishes so this would be an invalid command. Same for
any flash that goes into a stateful mode.
I see.
And you can't even keep using nor->read_proto to read SFDP because the
Read SFDP command might not be supported in all modes. xSPI spec
(JESD251) says that the Read SFDP command is optional in 8D-8D-8D mode.
I think the best approach for this would be to cache the entire SFDP
table at parse time. This obviously comes with a memory overhead but I
don't think it would be too big. For example, the sfdp table on
s28hs512t is 491 bytes and it has 6 tables. Anyway, if the memory usage
is too much of a problem we can put the feature behind a config.
I don't like to have it a config option, because then, if you really
need it, i.e. some user has an unknown flash, it might not be there.
The next question would be, should I leave the current parsing code
as is or should I also change that to use the sftp data cache. I'd
prefer to leave it as is.
-michael
Re: [RFC PATCH 1/3] mtd: spi-nor: sfdp: remember sfdp_size
Hi Pratyush,
Am 2021-03-16 11:42, schrieb Pratyush Yadav:
On 12/03/21 08:05PM, Michael Walle wrote:
Save the sftp_size in the spi_nor struct so we can use it to dump the
SFDP table without parsing the headers again.
Signed-off-by: Michael Walle
---
drivers/mtd/spi-nor/sfdp.c | 12
include/linux/mtd/spi-nor.h | 1 +
2 files changed, 13 insertions(+)
diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c
index 25142ec4737b..b1814afefc33 100644
--- a/drivers/mtd/spi-nor/sfdp.c
+++ b/drivers/mtd/spi-nor/sfdp.c
@@ -16,6 +16,7 @@
(((p)->parameter_table_pointer[2] << 16) | \
((p)->parameter_table_pointer[1] << 8) | \
((p)->parameter_table_pointer[0] << 0))
+#define SFDP_PARAM_HEADER_PARAM_LEN(p) ((p)->length * 4)
#define SFDP_BFPT_ID 0xff00 /* Basic Flash Parameter Table */
#define SFDP_SECTOR_MAP_ID 0xff81 /* Sector Map Table */
@@ -1263,6 +1264,7 @@ int spi_nor_parse_sfdp(struct spi_nor *nor,
struct sfdp_parameter_header *param_headers = NULL;
struct sfdp_header header;
struct device *dev = nor->dev;
+ size_t param_max_offset;
size_t psize;
int i, err;
@@ -1285,6 +1287,9 @@ int spi_nor_parse_sfdp(struct spi_nor *nor,
bfpt_header->major != SFDP_JESD216_MAJOR)
return -EINVAL;
+ nor->sfdp_size = SFDP_PARAM_HEADER_PTP(bfpt_header) +
+SFDP_PARAM_HEADER_PARAM_LEN(bfpt_header);
+
/*
* Allocate memory then read all parameter headers with a single
* Read SFDP command. These parameter headers will actually be
parsed
@@ -1311,6 +1316,13 @@ int spi_nor_parse_sfdp(struct spi_nor *nor,
}
}
+ for (i = 0; i < header.nph; i++) {
+ param_header = _headers[i];
+ param_max_offset = SFDP_PARAM_HEADER_PTP(param_header) +
+ SFDP_PARAM_HEADER_PARAM_LEN(param_header);
+ nor->sfdp_size = max(nor->sfdp_size, param_max_offset);
+ }
+
I don't see any mention in the SFDP spec (JESD216D-01) that parameter
tables have to be contiguous.
ch. 6.1, fig.10 looks like all the headers come after the initial SFDP
header. If that wasn't the case, how would you find the headers then?
Also ch. 6.3
"""All subsequent parameter headers need to be contiguous and may be
specified..."""
In fact, it says that "Parameter tables
may be located anywhere in the SFDP space. They do not need to
immediately follow the parameter headers".
The tables itself, yes, but not the headers for the tables.
But I guess we can just say
the sysfs entry exports the entire SFDP space instead of just the
tables
so that is OK.
This patch looks good to me other than the small nitpick that we can
merge this loop and the one below that tries to find the latest BFPT
version.
btw. I've also added an export for the jedec id and the flash name to
this patch, which will be included in the next version.
/*
* Check other parameter headers to get the latest revision of
* the basic flash parameter table.
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
index a0d572855444..a58118b8b002 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -404,6 +404,7 @@ struct spi_nor {
boolsst_write_second;
u32 flags;
enum spi_nor_cmd_extcmd_ext_type;
+ size_t sfdp_size;
Documentation for this variable missing.
const struct spi_nor_controller_ops *controller_ops;
--
2.20.1
--
-michael
Re: [PATCH v2] PCI: Fix Intel i210 by avoiding overlapping of BARs
Am 2021-02-01 23:20, schrieb Bjorn Helgaas:
On Mon, Feb 01, 2021 at 08:49:16PM +0100, Michael Walle wrote:
Am 2021-01-17 20:27, schrieb Michael Walle:
> Am 2021-01-16 00:57, schrieb Bjorn Helgaas:
> > On Wed, Jan 13, 2021 at 12:32:32AM +0100, Michael Walle wrote:
> > > Am 2021-01-12 23:58, schrieb Bjorn Helgaas:
> > > > On Sat, Jan 09, 2021 at 07:31:46PM +0100, Michael Walle wrote:
> > > > > Am 2021-01-08 22:20, schrieb Bjorn Helgaas:
> >
> > > > > > 3) If the Intel i210 is defective in how it handles an Expansion ROM
> > > > > > that overlaps another BAR, a quirk might be the right fix. But my
> > > > > > guess is the device is working correctly per spec and there's
> > > > > > something wrong in how firmware/Linux is assigning things. That
would
> > > > > > mean we need a more generic fix that's not a quirk and not tied to
the
> > > > > > Intel i210.
> > > > >
> > > > > Agreed, but as you already stated (and I've also found that in
> > > > > the PCI spec) the Expansion ROM address decoder can be shared by
> > > > > the other BARs and it shouldn't matter as long as the ExpROM BAR
> > > > > is disabled, which is the case here.
> > > >
> > > > My point is just that if this could theoretically affect devices
> > > > other than the i210, the fix should not be an i210-specific quirk.
> > > > I'll assume this is a general problem and wait for a generic PCI
> > > > core solution unless it's i210-specific.
> > >
> > > I guess the culprit here is that linux skips the programming of the
> > > BAR because of some broken Matrox card. That should have been a
> > > quirk instead, right? But I don't know if we want to change that, do
> > > we? How many other cards depend on that?
> >
> > Oh, right. There's definitely some complicated history there that
> > makes me a little scared to change things. But it's also unfortunate
> > if we have to pile quirks on top of quirks.
> >
> > > And still, how do we find out that the i210 is behaving correctly?
> > > In my opinion it is clearly not. You can change the ExpROM BAR value
> > > during runtime and it will start working (while keeping it
> > > disabled). Am I missing something here?
> >
> > I agree; if the ROM BAR is disabled, I don't think it should matter at
> > all what it contains, so this does look like an i210 defect.
> >
> > Would you mind trying the patch below? It should update the ROM BAR
> > value even when it is disabled. With the current pci_enable_rom()
> > code that doesn't rely on the value read from the BAR, I *think* this
> > should be safe even on the Matrox and similar devices.
>
> Your patch will fix my issue:
>
> Tested-by: Michael Walle
any news on this?
Thanks for the reminder. I was thinking this morning that I need to
get back to this. I'm trying to convince myself that doing this
wouldn't break the problem fixed by 755528c860b0 ("Ignore disabled ROM
resources at setup"). So far I haven't quite succeeded.
ping #2 ;)
-michael
Re: [PATCH 1/2] ASoC: simple-card-utils: Do not handle device clock
Am 2021-03-15 18:31, schrieb Sameer Pujar:
This reverts commit 1e30f642cf29 ("ASoC: simple-card-utils: Fix device
module clock"). The original patch ended up breaking following
platform,
which depends on set_sysclk() to configure internal PLL on wm8904 codec
and expects simple-card-utils to not update the MCLK rate.
-
"arch/arm64/boot/dts/freescale/fsl-ls1028a-kontron-sl28-var3-ads2.dts"
It would be best if codec takes care of setting MCLK clock via DAI
set_sysclk() callback.
Reported-by: Michael Walle
Suggested-by: Mark Brown
Suggested-by: Michael Walle
Fixes: 1e30f642cf29 ("ASoC: simple-card-utils: Fix device module
clock")
Signed-off-by: Sameer Pujar
Thanks!
Tested-by: Michael Walle
-michael
Re: [PATCH 1/3] ASoC: simple-card-utils: Fix device module clock
Am 2021-03-15 18:10, schrieb Sameer Pujar: Yes this is a problem, unfortunately I missed checking some of the simple-card examples. I wonder we should be specifically looking for "mclk" clock here. That would definitely help mitigate the problem but I really think it's cleaner and safer to just push this down to set_sysclk(). Understand now. I will push patches based on this. Thanks for the details. Please keep me on CC, I'm not subscribed to the sound/alsa mailinglist. Thanks, -michael
Re: [PATCH 1/3] ASoC: simple-card-utils: Fix device module clock
Am 2021-03-15 16:19, schrieb Sameer Pujar:
On 3/15/2021 5:35 PM, Michael Walle wrote:
External email: Use caution opening links or attachments
Am 2021-03-12 14:46, schrieb Mark Brown:
On Fri, Mar 12, 2021 at 01:30:02PM +0100, Michael Walle wrote:
The card calls set_sysclk(), which eventually ends up in the codec.
The codec therefore, could figure out if it needs to configure the
clock or if it can use its internal FLL.
Is that what you mean?
Yes.
But the set_sysclk() of the codec isn't even called, because the
card itself already tries to call clk_set_rate() on the Codec's
MCLK,
which returns with an error [0].
OK, so I think we need to push this down a level so that the clock
setting is implemented by the core/CODEC rather than by simple-card,
with the helpers being something the CODEC can opt out of.
Sameer, it looks like the proper fix should be to add the clock
support to your codec.
I agree that complicated clock relationships should be handled within
the codec itself, however MCLK rate setting depends on "mclk-fs"
factor and this property is specified as part of
simple-card/audio-graph-card codec subnode. Right now codec, in
general, does not have a way to know this. The set_sysclk() callback
takes rate argument and not the factor.
Why would you need the factor?
Moreover the same codec is
used by other platform vendors too and unless a new DT property is
added for codec, runtime MCLK update based on the scaling factor
cannot be supported.
Could you test the following:
diff --git a/sound/soc/codecs/rt5659.c b/sound/soc/codecs/rt5659.c
index 67f0ab817135..7fd41f51f856 100644
--- a/sound/soc/codecs/rt5659.c
+++ b/sound/soc/codecs/rt5659.c
@@ -3426,12 +3426,18 @@ static int rt5659_set_component_sysclk(struct
snd_soc_component *component, int
{
struct rt5659_priv *rt5659 =
snd_soc_component_get_drvdata(component);
unsigned int reg_val = 0;
+ int ret;
if (freq == rt5659->sysclk && clk_id == rt5659->sysclk_src)
return 0;
switch (clk_id) {
case RT5659_SCLK_S_MCLK:
+ ret = clk_set_rate(rt5659->mclk, freq);
+ if (ret)
+ return ret;
reg_val |= RT5659_SCLK_SRC_MCLK;
break;
case RT5659_SCLK_S_PLL1:
-michael
This would mean that we will be having two
methods to specify "mclk-fs" factor, one from
simple-card/audio-graph-card and one from respective codec nodes,
which does not seem ideal.
Also it does not seem consistent with the way we handle MCLK clock
based on where it is specified.
a) If specified in simple-card/audio-graph-card, MCLK clock
rate/enable/disable updates are allowed.
b) If specified in codec device node, it is not expected to touch
the MCLK clock. This patch tried to treat it the same way as (a) does.
Advantage of this is, all codec drivers need not explicitly handle
MCLK, instead it is done at a central place. The platforms which use
specific machine drivers do the same and that is why probably the
codec driver patch was never required. It is about just setting MCLK
clock as a factor of sample rate, whenever the factor is available. I
understand that it is breaking your use case, but I am not sure if the
usage of set_sysclk() is consistent. I mean, it takes the "freq"
argument. Does it refer to MCLK rate or system-clock (sysclk) rate?
MCLK and sysclk are not really the same when codec PLL is involved. So
I would like to understand clearly about what "freq" argument means.
Also when "mclk-fs" factor is specified, is it related to MCLK or
sysclk? My understanding is that it should be strictly viewed as
related to MCLK.
Does it help if a separate helper is used for audio-graph-card with
current change and reverting the simple-card to its previous state?
Morimoto-san, does it affect any other users of audio-graph-card?
I've also looked at other users of "simple-audio-card" and
it looks like they will break too. For example,
- arch/arm64/boot/dts/rockchip/rk3399.dtsi
If I'm not mistaken, this will try to set the first clock
of hdmi@ff94 there, which is "iahb".
- arch/arm/boot/dts/sun8i-a33.dtsi
There " CLK_BUS_CODEC" of codec@1c22e00 will be changed
And it doesn't stop there, it also sets the first clock
of the CPU endpoint, which I guess just works because .set_rate
is a noop for the most clocks which are used there.
Yes this is a problem, unfortunately I missed checking some of the
simple-card examples. I wonder we should be specifically looking for
"mclk" clock here.
Re: [PATCH 1/3] ASoC: simple-card-utils: Fix device module clock
Am 2021-03-12 14:46, schrieb Mark Brown: On Fri, Mar 12, 2021 at 01:30:02PM +0100, Michael Walle wrote: The card calls set_sysclk(), which eventually ends up in the codec. The codec therefore, could figure out if it needs to configure the clock or if it can use its internal FLL. Is that what you mean? Yes. But the set_sysclk() of the codec isn't even called, because the card itself already tries to call clk_set_rate() on the Codec's MCLK, which returns with an error [0]. OK, so I think we need to push this down a level so that the clock setting is implemented by the core/CODEC rather than by simple-card, with the helpers being something the CODEC can opt out of. Sameer, it looks like the proper fix should be to add the clock support to your codec. I've also looked at other users of "simple-audio-card" and it looks like they will break too. For example, - arch/arm64/boot/dts/rockchip/rk3399.dtsi If I'm not mistaken, this will try to set the first clock of hdmi@ff94 there, which is "iahb". - arch/arm/boot/dts/sun8i-a33.dtsi There " CLK_BUS_CODEC" of codec@1c22e00 will be changed And it doesn't stop there, it also sets the first clock of the CPU endpoint, which I guess just works because .set_rate is a noop for the most clocks which are used there. -michael
Re: [PATCH v4 2/4] mtd: spi-nor: implement OTP support for Winbond and similar flashes
Am 2021-03-15 09:31, schrieb tudor.amba...@microchip.com: On 3/6/21 2:05 AM, Michael Walle wrote: + nor->dirmap.rdesc = NULL; why can't we use dirmap? Dirmap is used if the controller supports (transparent) memory mapped access, right? As you see I'm not familiar with that, nor does my controller has support for it, well at least the driver, the controller supports it actually. But it also seems like how the flash is accessed is set up in spi_nor_probe() spi_nor_create_read_dirmap() And because the read opcode has to be changed, that isn't possible. Plese correct me if I'm wrong. -michael
