subject:"\[PATCH v6 15\/22\] drivers\: use dev_read_addr_index

Re: [PATCH v6 15/22] drivers: use dev_read_addr_index_ptr when cast to pointer

2023-03-11 Thread Simon Glass

Hi Johan,

On Fri, 10 Mar 2023 at 17:37, Simon Glass  wrote:
>
> Hi Johan,
>
> On Mon, 6 Mar 2023 at 12:32, Johan Jonker  wrote:
> >
> >
> >
> > On 3/6/23 19:20, Simon Glass wrote:
> > > Hi Johan,
> > >
> > > On Thu, 2 Mar 2023 at 17:15, Johan Jonker  wrote:
> > >>
> > >> The fdt_addr_t and phys_addr_t size have been decoupled. A 32bit CPU
> > >> can expect 64-bit data from the device tree parser, so use
> > >
> > > Why is that? It seems quite inefficient.
> > 1:
> > ===
> > Because the device tree does describes more then just only the internal 
> > io/mem range.
> > When a NAND chip is connected it must be able to describe partitions far 
> > beyond that 32bit range.
> > This change only changes the PARSE capacity defined by fdt_addr_t and 
> > fdt_size_t and not the phys_addr_t and phys_size_t.
> > Most drivers make a little mess when taking a DT reg value and then trying 
> > to make it fit in a structure of multiple registers with various offsets.
> > Fixing all of that is beyond my capacity/this serie and more a MAINTAINER 
> > task.
> >
> > https://elixir.bootlin.com/u-boot/latest/source/drivers/mtd/mtdpart.c#L933
> >
> > struct mtd_partition {
> > const char *name;   /* identifier string */
> > uint64_t size;  /* partition size */
> > uint64_t offset;/* offset within the master MTD 
> > space */
> > uint32_t mask_flags;/* master MTD flags to mask out for 
> > this partition */
> > struct nand_ecclayout *ecclayout;   /* out of band layout for 
> > this partition (NAND only) */
> > };
> >
> > int add_mtd_partitions_of(struct mtd_info *master)
> > struct mtd_partition part = { 0 };
> > fdt_addr_t offset;
> > fdt_size_t size;
> >
> > [..]
> > part.offset = offset;
> > part.size = size;
> >
> > While the mtd_partition structure is ready with uint64_t size all the parse 
> > functions that export this reg value were typedef to a 32bit value.
> >
> > ===
> > Given mk808 rk3066a with NAND:
> >
> > [   38.750789] nand: Hynix H27UCG8T2ATR-BC 64G 3.3V 8-bit
> > [   38.756650] nand: 8192 MiB, MLC, erase size: 2048 KiB, page size: 8192, 
> > OOB size: 640
> >
> > ===
> > BEFORE:
> >
> > List of MTD devices:
> > * nand0
> >   - type: MLC NAND flash
> >   - block size: 0x20 bytes
> >   - min I/O: 0x2000 bytes
> >   - OOB size: 640 bytes
> >   - OOB available: 4294967290 bytes
> >   - ECC strength: 40 bits
> >   - ECC step size: 1024 bytes
> >   - bitflip threshold: 30 bits
> >   - 0x-0x0002 : "nand0"
> >   - 0x0040-0x0060 : "boot-blk-0"
> >   - 0x0060-0x0080 : "boot-blk-1"
> >   - 0x0080-0x00a0 : "boot-blk-2"
> >   - 0x00a0-0x00c0 : "boot-blk-3"
> >   - 0x00c0-0x00e0 : "boot-blk-4"
> >   - 0x0100-0xfe00 : "rootfs"
> >   - 0xfe00-0x0001 : "bbt"
> >
> > # This output is corrupted.
> >
> > ===
> > AFTER:
> >
> > List of MTD devices:
> > * nand0
> >   - type: MLC NAND flash
> >   - block size: 0x20 bytes
> >   - min I/O: 0x2000 bytes
> >   - OOB size: 640 bytes
> >   - OOB available: 4294967290 bytes
> >   - ECC strength: 40 bits
> >   - ECC step size: 1024 bytes
> >   - bitflip threshold: 30 bits
> >   - 0x-0x0002 : "nand0"
> >   - 0x0040-0x0060 : "boot-blk-0"
> >   - 0x0060-0x0080 : "boot-blk-1"
> >   - 0x0080-0x00a0 : "boot-blk-2"
> >   - 0x00a0-0x00c0 : "boot-blk-3"
> >   - 0x00c0-0x00e0 : "boot-blk-4"
> >   - 0x0100-0x0001fe00 : "rootfs"
> >   - 0x0001fe00-0x0002 : "bbt"
> > ===
> > Example:
> > partitions {
> > compatible = "fixed-partitions";
> > #address-cells = <2>;
> > #size-cells = <2>;
> >
> > partition@40 {
> > reg = <0x0 0x0040 0x0 0x0020>;
> > label = "boot-blk-0";
> > };
> >
> > partition@60 {
> > reg = <0x0 0x0060 0x0 0x0020>;
> > label = "boot-blk-1";
> > };
> >
> > partition@80 {
> > reg = <0x0 0x0080 0x0 0x0020>;
> > label = "boot-blk-2";
> > };
> >
> > partition@a0 {
> > reg = <0x0 0x00a0 0x0 0x0020>;
> > label = "boot-blk-3";
> > };
> >
> >

Re: [PATCH v6 15/22] drivers: use dev_read_addr_index_ptr when cast to pointer

2023-03-10 Thread Simon Glass

Hi Johan,

On Mon, 6 Mar 2023 at 12:32, Johan Jonker  wrote:
>
>
>
> On 3/6/23 19:20, Simon Glass wrote:
> > Hi Johan,
> >
> > On Thu, 2 Mar 2023 at 17:15, Johan Jonker  wrote:
> >>
> >> The fdt_addr_t and phys_addr_t size have been decoupled. A 32bit CPU
> >> can expect 64-bit data from the device tree parser, so use
> >
> > Why is that? It seems quite inefficient.
> 1:
> ===
> Because the device tree does describes more then just only the internal 
> io/mem range.
> When a NAND chip is connected it must be able to describe partitions far 
> beyond that 32bit range.
> This change only changes the PARSE capacity defined by fdt_addr_t and 
> fdt_size_t and not the phys_addr_t and phys_size_t.
> Most drivers make a little mess when taking a DT reg value and then trying to 
> make it fit in a structure of multiple registers with various offsets.
> Fixing all of that is beyond my capacity/this serie and more a MAINTAINER 
> task.
>
> https://elixir.bootlin.com/u-boot/latest/source/drivers/mtd/mtdpart.c#L933
>
> struct mtd_partition {
> const char *name;   /* identifier string */
> uint64_t size;  /* partition size */
> uint64_t offset;/* offset within the master MTD space 
> */
> uint32_t mask_flags;/* master MTD flags to mask out for 
> this partition */
> struct nand_ecclayout *ecclayout;   /* out of band layout for 
> this partition (NAND only) */
> };
>
> int add_mtd_partitions_of(struct mtd_info *master)
> struct mtd_partition part = { 0 };
> fdt_addr_t offset;
> fdt_size_t size;
>
> [..]
> part.offset = offset;
> part.size = size;
>
> While the mtd_partition structure is ready with uint64_t size all the parse 
> functions that export this reg value were typedef to a 32bit value.
>
> ===
> Given mk808 rk3066a with NAND:
>
> [   38.750789] nand: Hynix H27UCG8T2ATR-BC 64G 3.3V 8-bit
> [   38.756650] nand: 8192 MiB, MLC, erase size: 2048 KiB, page size: 8192, 
> OOB size: 640
>
> ===
> BEFORE:
>
> List of MTD devices:
> * nand0
>   - type: MLC NAND flash
>   - block size: 0x20 bytes
>   - min I/O: 0x2000 bytes
>   - OOB size: 640 bytes
>   - OOB available: 4294967290 bytes
>   - ECC strength: 40 bits
>   - ECC step size: 1024 bytes
>   - bitflip threshold: 30 bits
>   - 0x-0x0002 : "nand0"
>   - 0x0040-0x0060 : "boot-blk-0"
>   - 0x0060-0x0080 : "boot-blk-1"
>   - 0x0080-0x00a0 : "boot-blk-2"
>   - 0x00a0-0x00c0 : "boot-blk-3"
>   - 0x00c0-0x00e0 : "boot-blk-4"
>   - 0x0100-0xfe00 : "rootfs"
>   - 0xfe00-0x0001 : "bbt"
>
> # This output is corrupted.
>
> ===
> AFTER:
>
> List of MTD devices:
> * nand0
>   - type: MLC NAND flash
>   - block size: 0x20 bytes
>   - min I/O: 0x2000 bytes
>   - OOB size: 640 bytes
>   - OOB available: 4294967290 bytes
>   - ECC strength: 40 bits
>   - ECC step size: 1024 bytes
>   - bitflip threshold: 30 bits
>   - 0x-0x0002 : "nand0"
>   - 0x0040-0x0060 : "boot-blk-0"
>   - 0x0060-0x0080 : "boot-blk-1"
>   - 0x0080-0x00a0 : "boot-blk-2"
>   - 0x00a0-0x00c0 : "boot-blk-3"
>   - 0x00c0-0x00e0 : "boot-blk-4"
>   - 0x0100-0x0001fe00 : "rootfs"
>   - 0x0001fe00-0x0002 : "bbt"
> ===
> Example:
> partitions {
> compatible = "fixed-partitions";
> #address-cells = <2>;
> #size-cells = <2>;
>
> partition@40 {
> reg = <0x0 0x0040 0x0 0x0020>;
> label = "boot-blk-0";
> };
>
> partition@60 {
> reg = <0x0 0x0060 0x0 0x0020>;
> label = "boot-blk-1";
> };
>
> partition@80 {
> reg = <0x0 0x0080 0x0 0x0020>;
> label = "boot-blk-2";
> };
>
> partition@a0 {
> reg = <0x0 0x00a0 0x0 0x0020>;
> label = "boot-blk-3";
> };
>
> partition@c0 {
> reg = <0x0 0x00c0 0x0 0x0020>;
> label = "boot-blk-4";
> };
>
> partition@100 {
> reg = <0x0 0x0100 0x1 0xfd00>;
> label =

Re: [PATCH v6 15/22] drivers: use dev_read_addr_index_ptr when cast to pointer

2023-03-06 Thread Johan Jonker




On 3/6/23 19:20, Simon Glass wrote:
> Hi Johan,
> 
> On Thu, 2 Mar 2023 at 17:15, Johan Jonker  wrote:
>>
>> The fdt_addr_t and phys_addr_t size have been decoupled. A 32bit CPU
>> can expect 64-bit data from the device tree parser, so use
> 
> Why is that? It seems quite inefficient.
1:
===
Because the device tree does describes more then just only the internal io/mem 
range.
When a NAND chip is connected it must be able to describe partitions far beyond 
that 32bit range.
This change only changes the PARSE capacity defined by fdt_addr_t and 
fdt_size_t and not the phys_addr_t and phys_size_t.
Most drivers make a little mess when taking a DT reg value and then trying to 
make it fit in a structure of multiple registers with various offsets.
Fixing all of that is beyond my capacity/this serie and more a MAINTAINER task.

https://elixir.bootlin.com/u-boot/latest/source/drivers/mtd/mtdpart.c#L933

struct mtd_partition {
const char *name;   /* identifier string */
uint64_t size;  /* partition size */
uint64_t offset;/* offset within the master MTD space */
uint32_t mask_flags;/* master MTD flags to mask out for 
this partition */
struct nand_ecclayout *ecclayout;   /* out of band layout for this 
partition (NAND only) */
};

int add_mtd_partitions_of(struct mtd_info *master)
struct mtd_partition part = { 0 };
fdt_addr_t offset;
fdt_size_t size;

[..]
part.offset = offset;
part.size = size;

While the mtd_partition structure is ready with uint64_t size all the parse 
functions that export this reg value were typedef to a 32bit value.

===
Given mk808 rk3066a with NAND:

[   38.750789] nand: Hynix H27UCG8T2ATR-BC 64G 3.3V 8-bit
[   38.756650] nand: 8192 MiB, MLC, erase size: 2048 KiB, page size: 8192, OOB 
size: 640

===
BEFORE:

List of MTD devices:
* nand0
  - type: MLC NAND flash
  - block size: 0x20 bytes
  - min I/O: 0x2000 bytes
  - OOB size: 640 bytes
  - OOB available: 4294967290 bytes
  - ECC strength: 40 bits
  - ECC step size: 1024 bytes
  - bitflip threshold: 30 bits
  - 0x-0x0002 : "nand0"
  - 0x0040-0x0060 : "boot-blk-0"
  - 0x0060-0x0080 : "boot-blk-1"
  - 0x0080-0x00a0 : "boot-blk-2"
  - 0x00a0-0x00c0 : "boot-blk-3"
  - 0x00c0-0x00e0 : "boot-blk-4"
  - 0x0100-0xfe00 : "rootfs"
  - 0xfe00-0x0001 : "bbt"

# This output is corrupted.

===
AFTER:

List of MTD devices:
* nand0
  - type: MLC NAND flash
  - block size: 0x20 bytes
  - min I/O: 0x2000 bytes
  - OOB size: 640 bytes
  - OOB available: 4294967290 bytes
  - ECC strength: 40 bits
  - ECC step size: 1024 bytes
  - bitflip threshold: 30 bits
  - 0x-0x0002 : "nand0"
  - 0x0040-0x0060 : "boot-blk-0"
  - 0x0060-0x0080 : "boot-blk-1"
  - 0x0080-0x00a0 : "boot-blk-2"
  - 0x00a0-0x00c0 : "boot-blk-3"
  - 0x00c0-0x00e0 : "boot-blk-4"
  - 0x0100-0x0001fe00 : "rootfs"
  - 0x0001fe00-0x0002 : "bbt"
===
Example:
partitions {
compatible = "fixed-partitions";
#address-cells = <2>;
#size-cells = <2>;

partition@40 {
reg = <0x0 0x0040 0x0 0x0020>;
label = "boot-blk-0";
};

partition@60 {
reg = <0x0 0x0060 0x0 0x0020>;
label = "boot-blk-1";
};

partition@80 {
reg = <0x0 0x0080 0x0 0x0020>;
label = "boot-blk-2";
};

partition@a0 {
reg = <0x0 0x00a0 0x0 0x0020>;
label = "boot-blk-3";
};

partition@c0 {
reg = <0x0 0x00c0 0x0 0x0020>;
label = "boot-blk-4";
};

partition@100 {
reg = <0x0 0x0100 0x1 0xfd00>;
label = "rootfs";
};

partition@1fe00 {
reg = <0x1 0xfe00 0x0 0x0200>;
label = "bbt";
};
};
===

2:
As a side effect Rockchip rk3288 with 32bit reg should be

Re: [PATCH v6 15/22] drivers: use dev_read_addr_index_ptr when cast to pointer

2023-03-06 Thread Simon Glass

Hi Johan,

On Thu, 2 Mar 2023 at 17:15, Johan Jonker  wrote:
>
> The fdt_addr_t and phys_addr_t size have been decoupled. A 32bit CPU
> can expect 64-bit data from the device tree parser, so use

Why is that? It seems quite inefficient.

> dev_read_addr_index_ptr instead of the dev_read_addr_index function
> in the various files in the drivers directory that cast to a pointer.
>
> Signed-off-by: Johan Jonker 
> Reviewed-by: Michael Trimarchi 
> ---
>
> Changed V6:
>   use -EINVAL on return
>   drop cast
> ---
>  drivers/mtd/nand/raw/cortina_nand.c |  4 ++--
>  drivers/net/dm9000x.c   |  2 +-
>  drivers/net/dwmac_meson8b.c |  4 ++--
>  drivers/pci/pcie_dw_meson.c |  8 
>  drivers/pci/pcie_dw_rockchip.c  |  8 
>  drivers/watchdog/sbsa_gwdt.c| 12 ++--
>  6 files changed, 19 insertions(+), 19 deletions(-)

[..]

Regards,
SImon

[PATCH v6 15/22] drivers: use dev_read_addr_index_ptr when cast to pointer

2023-03-02 Thread Johan Jonker

The fdt_addr_t and phys_addr_t size have been decoupled. A 32bit CPU
can expect 64-bit data from the device tree parser, so use
dev_read_addr_index_ptr instead of the dev_read_addr_index function
in the various files in the drivers directory that cast to a pointer.

Signed-off-by: Johan Jonker 
Reviewed-by: Michael Trimarchi 
---

Changed V6:
  use -EINVAL on return
  drop cast
---
 drivers/mtd/nand/raw/cortina_nand.c |  4 ++--
 drivers/net/dm9000x.c   |  2 +-
 drivers/net/dwmac_meson8b.c |  4 ++--
 drivers/pci/pcie_dw_meson.c |  8 
 drivers/pci/pcie_dw_rockchip.c  |  8 
 drivers/watchdog/sbsa_gwdt.c| 12 ++--
 6 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/drivers/mtd/nand/raw/cortina_nand.c 
b/drivers/mtd/nand/raw/cortina_nand.c
index 88798f23..8de35731 100644
--- a/drivers/mtd/nand/raw/cortina_nand.c
+++ b/drivers/mtd/nand/raw/cortina_nand.c
@@ -1175,8 +1175,8 @@ static int fdt_decode_nand(struct udevice *dev, struct 
nand_drv *info)
int ecc_strength;

info->reg = (struct nand_ctlr *)dev_read_addr(dev);
-   info->dma_glb = (struct dma_global *)dev_read_addr_index(dev, 1);
-   info->dma_nand = (struct dma_ssp *)dev_read_addr_index(dev, 2);
+   info->dma_glb = dev_read_addr_index_ptr(dev, 1);
+   info->dma_nand = dev_read_addr_index_ptr(dev, 2);
info->config.enabled = dev_read_enabled(dev);
ecc_strength = dev_read_u32_default(dev, "nand-ecc-strength", 16);
info->flash_base =
diff --git a/drivers/net/dm9000x.c b/drivers/net/dm9000x.c
index b46bdeb2..bec8d67d 100644
--- a/drivers/net/dm9000x.c
+++ b/drivers/net/dm9000x.c
@@ -651,7 +651,7 @@ static int dm9000_of_to_plat(struct udevice *dev)

pdata->iobase = dev_read_addr_index(dev, 0);
db->base_io = (void __iomem *)pdata->iobase;
-   db->base_data = (void __iomem *)dev_read_addr_index(dev, 1);
+   db->base_data = dev_read_addr_index_ptr(dev, 1);

return 0;
 }
diff --git a/drivers/net/dwmac_meson8b.c b/drivers/net/dwmac_meson8b.c
index ddbaa87d..871171e1 100644
--- a/drivers/net/dwmac_meson8b.c
+++ b/drivers/net/dwmac_meson8b.c
@@ -41,8 +41,8 @@ static int dwmac_meson8b_of_to_plat(struct udevice *dev)
 {
struct dwmac_meson8b_plat *pdata = dev_get_plat(dev);

-   pdata->regs = (void *)dev_read_addr_index(dev, 1);
-   if ((fdt_addr_t)pdata->regs == FDT_ADDR_T_NONE)
+   pdata->regs = dev_read_addr_index_ptr(dev, 1);
+   if (!pdata->regs)
return -EINVAL;

pdata->dwmac_setup = (void *)dev_get_driver_data(dev);
diff --git a/drivers/pci/pcie_dw_meson.c b/drivers/pci/pcie_dw_meson.c
index 07da9fa5..f9537979 100644
--- a/drivers/pci/pcie_dw_meson.c
+++ b/drivers/pci/pcie_dw_meson.c
@@ -337,15 +337,15 @@ static int meson_pcie_parse_dt(struct udevice *dev)
struct meson_pcie *priv = dev_get_priv(dev);
int ret;

-   priv->dw.dbi_base = (void *)dev_read_addr_index(dev, 0);
+   priv->dw.dbi_base = dev_read_addr_index_ptr(dev, 0);
if (!priv->dw.dbi_base)
-   return -ENODEV;
+   return -EINVAL;

dev_dbg(dev, "ELBI address is 0x%p\n", priv->dw.dbi_base);

-   priv->meson_cfg_base = (void *)dev_read_addr_index(dev, 1);
+   priv->meson_cfg_base = dev_read_addr_index_ptr(dev, 1);
if (!priv->meson_cfg_base)
-   return -ENODEV;
+   return -EINVAL;

dev_dbg(dev, "CFG address is 0x%p\n", priv->meson_cfg_base);

diff --git a/drivers/pci/pcie_dw_rockchip.c b/drivers/pci/pcie_dw_rockchip.c
index 9322e735..624ca1cb 100644
--- a/drivers/pci/pcie_dw_rockchip.c
+++ b/drivers/pci/pcie_dw_rockchip.c
@@ -353,15 +353,15 @@ static int rockchip_pcie_parse_dt(struct udevice *dev)
struct rk_pcie *priv = dev_get_priv(dev);
int ret;

-   priv->dw.dbi_base = (void *)dev_read_addr_index(dev, 0);
+   priv->dw.dbi_base = dev_read_addr_index_ptr(dev, 0);
if (!priv->dw.dbi_base)
-   return -ENODEV;
+   return -EINVAL;

dev_dbg(dev, "DBI address is 0x%p\n", priv->dw.dbi_base);

-   priv->apb_base = (void *)dev_read_addr_index(dev, 1);
+   priv->apb_base = dev_read_addr_index_ptr(dev, 1);
if (!priv->apb_base)
-   return -ENODEV;
+   return -EINVAL;

dev_dbg(dev, "APB address is 0x%p\n", priv->apb_base);

diff --git a/drivers/watchdog/sbsa_gwdt.c b/drivers/watchdog/sbsa_gwdt.c
index f43cd3fd..96d04665 100644
--- a/drivers/watchdog/sbsa_gwdt.c
+++ b/drivers/watchdog/sbsa_gwdt.c
@@ -98,13 +98,13 @@ static int sbsa_gwdt_of_to_plat(struct udevice *dev)
 {
struct sbsa_gwdt_priv *priv = dev_get_priv(dev);

-   priv->reg_control = (void __iomem *)dev_read_addr_index(dev, 0);
-   if (IS_ERR(priv->reg_control))
-   return PTR_ERR(priv->reg_control);
+   priv->reg_control = dev_read_addr_index_ptr(dev, 0);
+   if (!priv->reg_control)
+

Re: [PATCH v6 15/22] drivers: use dev_read_addr_index_ptr when cast to pointer

Re: [PATCH v6 15/22] drivers: use dev_read_addr_index_ptr when cast to pointer

Re: [PATCH v6 15/22] drivers: use dev_read_addr_index_ptr when cast to pointer

Re: [PATCH v6 15/22] drivers: use dev_read_addr_index_ptr when cast to pointer

[PATCH v6 15/22] drivers: use dev_read_addr_index_ptr when cast to pointer

5 matches

Site Navigation

Mail list logo

Footer information