Re: [RFC 14/22] efi_loader: disk: a helper function to create efi_disk objects from udevice
On Sun, Oct 10, 2021 at 08:14:21AM -0600, Simon Glass wrote:
> On Thu, 30 Sept 2021 at 23:04, AKASHI Takahiro
> wrote:
> >
> > Add efi_disk_create() function.
> >
> > Any UEFI handle created by efi_disk_create() can be treated as a efi_disk
> > object, the udevice is either a UCLASS_BLK (a whole raw disk) or
> > UCLASS_PARTITION (a disk partition).
> >
> > So this function is expected to be called every time such an udevice
> > is detected and activated through a device model's "probe" interface.
> >
> > Signed-off-by: AKASHI Takahiro
> > ---
> > include/efi_loader.h | 2 +
> > lib/efi_loader/efi_disk.c | 92 +++
> > 2 files changed, 94 insertions(+)
> >
>
> Reviewed-by: Simon Glass
>
> But some nits below.
>
> Don't worry about !CONFIG_BLK - that code should be removed.
Yes. I added a tentative patch to remove !CONFIG_BLK code in efi_disk
in patch#13.
> > diff --git a/include/efi_loader.h b/include/efi_loader.h
> > index c440962fe522..751fde7fb153 100644
> > --- a/include/efi_loader.h
> > +++ b/include/efi_loader.h
> > @@ -517,6 +517,8 @@ efi_status_t EFIAPI efi_convert_pointer(efi_uintn_t
> > debug_disposition,
> > void efi_carve_out_dt_rsv(void *fdt);
> > /* Called by bootefi to make console interface available */
> > efi_status_t efi_console_register(void);
> > +/* Called when a block devices has been probed */
> > +int efi_disk_create(struct udevice *dev);
>
> Please buck the trend in this file and add a full function comment. In
> this case it needs to cover @dev and the return value and also explain
> what the function does.
OK.
> > /* Called by bootefi to make all disk storage accessible as EFI objects */
> > efi_status_t efi_disk_register(void);
> > /* Called by efi_init_obj_list() to install EFI_RNG_PROTOCOL */
> > diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
> > index cd5528046251..3fae40e034fb 100644
> > --- a/lib/efi_loader/efi_disk.c
> > +++ b/lib/efi_loader/efi_disk.c
> > @@ -10,6 +10,7 @@
> > #include
> > #include
> > #include
> > +#include
> > #include
> > #include
> > #include
> > @@ -484,6 +485,7 @@ error:
> > return ret;
> > }
> >
> > +#ifndef CONFIG_BLK
> > /**
> > * efi_disk_create_partitions() - create handles and protocols for
> > partitions
> > *
> > @@ -531,6 +533,96 @@ int efi_disk_create_partitions(efi_handle_t parent,
> > struct blk_desc *desc,
> >
> > return disks;
> > }
> > +#endif /* CONFIG_BLK */
> > +
> > +/*
> > + * Create a handle for a whole raw disk
> > + *
> > + * @devuclass device
>
> ?? what type of device?
(Will fix: UCLASS_BLK)
> > + * @return 0 on success, -1 otherwise
> > + */
> > +static int efi_disk_create_raw(struct udevice *dev)
> > +{
> > + struct efi_disk_obj *disk;
> > + struct blk_desc *desc;
> > + const char *if_typename;
> > + int diskid;
> > + efi_status_t ret;
> > +
> > + desc = dev_get_uclass_plat(dev);
> > + if_typename = blk_get_if_type_name(desc->if_type);
> > + diskid = desc->devnum;
> > +
> > + ret = efi_disk_add_dev(NULL, NULL, if_typename, desc,
> > + diskid, NULL, 0, &disk);
> > + if (ret != EFI_SUCCESS) {
>
> if (ret)
>
> is much shorter and easier to read
Yeah, but I don't want to assume EFI_SUCCESS is *zero*.
> > + log_err("Adding disk %s%d failed\n", if_typename, diskid);
> > + return -1;
> > + }
> > + disk->dev = dev;
> > + dev->efi_obj = &disk->header;
> > +
> > + return 0;
> > +}
> > +
> > +/*
> > + * Create a handle for a disk partition
> > + *
> > + * @devuclass device
>
> ??
(UCLASS_PARTITION)
> > + * @return 0 on success, -1 otherwise
> > + */
> > +static int efi_disk_create_part(struct udevice *dev)
> > +{
> > + efi_handle_t parent;
> > + struct blk_desc *desc;
> > + const char *if_typename;
> > + struct disk_part *part_data;
> > + struct disk_partition *info;
> > + unsigned int part;
> > + int diskid;
> > + struct efi_device_path *dp = NULL;
> > + struct efi_disk_obj *disk;
> > + efi_status_t ret;
> > +
> > + parent = dev->parent->efi_obj;
>
> dev_get_parent(dev)
I will replace all of "dev->parent" with dev_get_parent()
if I will not forget to do that in the next version :)
Thanks,
-Takahiro Akashi
> > + desc = dev_get_uclass_plat(dev->parent);
> > + if_typename = blk_get_if_type_name(desc->if_type);
> > + diskid = desc->devnum;
> > +
> > + part_data = dev_get_uclass_plat(dev);
> > + part = part_data->partnum;
> > + info = &part_data->gpt_part_info;
> > +
> > + /* TODO: should not use desc? */
> > + dp = efi_dp_from_part(desc, 0);
> > +
> > + ret = efi_disk_add_dev(parent, dp, if_typename, desc, diskid,
> > + info, part, &disk);
> > + if (ret != EFI_SUCCESS) {
> >
Re: [RFC 12/22] dm: add a hidden link to efi object
Simon,
On Sun, Oct 10, 2021 at 08:14:18AM -0600, Simon Glass wrote:
> Hi Takahiro,
>
> On Thu, 30 Sept 2021 at 23:04, AKASHI Takahiro
> wrote:
> >
> > This member field in udevice will be used to dereference from udevice
> > to efi_object (or efi_handle).
> >
> > Signed-off-by: AKASHI Takahiro
> > ---
> > include/dm/device.h | 4
> > 1 file changed, 4 insertions(+)
>
> I think this should be generalised.
>
> Can we add a simple API for attaching things to devices? Something like:
Ok.
> config DM_TAG
>bool "Support tags attached to devices"
>
> enum dm_tag_t {
> DM_TAG_EFI = 0,
>
> DM_TAG_COUNT,
> };
>
> ret = dev_tag_set_ptr(dev, DM_TAG_EFI, ptr);
>
> void *ptr = dev_tag_get_ptr(dev, DM_TAG_EFI);
>
> ulong val = dev_tag_get_val(dev, DM_TAG_EFI);
>
> Under the hood I think for now we could have a simple list of tags for
> all of DM:
>
> struct dmtag_node {
>struct list_head sibling;
>struct udevice *dev;
>enum dm_tag_t tag;
>union {
> void *ptr;
> ulong val;
> };
> };
Just let me make sure; Do you intend that we have a *single* list of tags
in the system instead of maintaining a list *per udevice*?
-Takahiro Akashi
> This can be useful in other situations, for example I think we need to
> be able to send an event when a device is probed so that other devices
> (with tags attached) can take action. But in any case, it makes the
> API separate from the data structure, so aids refactoring later.
>
> If we find that this is slow we can change the impl, but I doubt it
> will matter fornow.
>
> >
> > diff --git a/include/dm/device.h b/include/dm/device.h
> > index 0a9718a5b81a..33b09a836f06 100644
> > --- a/include/dm/device.h
> > +++ b/include/dm/device.h
> > @@ -190,6 +190,10 @@ struct udevice {
> > #if CONFIG_IS_ENABLED(DM_DMA)
> > ulong dma_offset;
> > #endif
> > +#if CONFIG_IS_ENABLED(EFI_LOADER)
> > + /* link to efi_object */
> > + void *efi_obj;
> > +#endif
> > };
> >
> > /**
> > --
> > 2.33.0
> >
>
> Regards,
> Simon
Re: [PATCH] tools: Stop re-defining -std= when building tools
On Mon, Oct 11, 2021 at 3:23 AM Tom Rini wrote: > > While we intentionally set -std=gnu11 for building host tools, and have > for quite some time, we never dropped -std=gnu99 from tools/Makefile. > This resulted in passing -std=gnu11 ... -std=gnu99 when building, and > gnu99 would win. This in turn would result now in warnings such as: > tools/mkeficapsule.c:25:15: warning: redefinition of typedef 'u32' is a C11 > feature [-Wtypedef-redefinition] > typedef __u32 u32; > ^ > > Signed-off-by: Tom Rini > --- > tools/Makefile | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/tools/Makefile b/tools/Makefile > index 999fd4653166..b45219e2c30c 100644 > --- a/tools/Makefile > +++ b/tools/Makefile > @@ -295,8 +295,7 @@ HOST_EXTRACFLAGS += -include > $(srctree)/include/compiler.h \ > -I$(srctree)/tools \ > -DUSE_HOSTCC \ > -D__KERNEL_STRICT_NAMES \ > - -D_GNU_SOURCE \ > - -std=gnu99 > + -D_GNU_SOURCE It looks like std=gnu11 is only added for Linux host. KBUILD_HOSTCFLAGS += $(CSTD_FLAG) Should we still keep it for other hosts? Regards, Bin
Re: [RFC 14/22] dm: blk: call efi's device-probe hook
Hi Simon,
On Sun, Oct 10, 2021 at 08:14:23AM -0600, Simon Glass wrote:
> Hi Takahiro,
>
> On Thu, 30 Sept 2021 at 23:04, AKASHI Takahiro
> wrote:
> >
> > Adding this callback function, efi_disk_create() in block devices's
> > post_probe hook will allows for automatically creating efi_disk objects
> > per block device.
> >
> > This will end up not only eliminating efi_disk_register() called in UEFI
> > initialization, but also enabling detections of new block devices even
> > after the initialization.
> >
> > Signed-off-by: AKASHI Takahiro
> > ---
> > drivers/block/blk-uclass.c | 10 ++
> > 1 file changed, 10 insertions(+)
>
> This is where events come in. We need a way to notify things when
> devices go through different stages, etc.
I favor your idea of event notification to decouple subsystems from
the core block layer.
> I am thinking of:
>
> enum dm_event_t {
>DMEVT_PROBE,
>DMEVT_POST_PROBE,
> ...
> };
>
> struct dm_event {
> enum dm_event_t type;
> union {
> // add data for different event types in here
> } data;
> }
>
> int (*dm_handler_t)(void *ctx, struct dm_event *event);
>
> int dm_register(enum dm_event_t evtype, dm_handler_t func, void *ctx)
>
> int dm_notify(struct udevice *dev, enum dm_event_t type, void *data);
>
> Then the code below becomes:
>
> dm_notify(struct udevice *dev, DMEVT_POST_PROBE, NULL);
>
> the implementation of which will call the handler.
>
> If you like I could create an impl of the above as a separate patch
> for discussion.
Yes, please.
I'm willing to rebase my code on top of your patch.
-Takahiro Akashi
> >
> > diff --git a/drivers/block/blk-uclass.c b/drivers/block/blk-uclass.c
> > index 8fbec8779e1e..ce45cf0a8768 100644
> > --- a/drivers/block/blk-uclass.c
> > +++ b/drivers/block/blk-uclass.c
> > @@ -9,6 +9,7 @@
> > #include
> > #include
> > #include
> > +#include
> > #include
> > #include
> > #include
> > @@ -827,6 +828,11 @@ int blk_create_partitions(struct udevice *parent)
> >
> > static int blk_post_probe(struct udevice *dev)
> > {
> > + if (CONFIG_IS_ENABLED(EFI_LOADER)) {
> > + if (efi_disk_create(dev))
> > + debug("*** efi_post_probe_device failed\n");
> > + }
> > +
> > if (IS_ENABLED(CONFIG_PARTITIONS) &&
> > IS_ENABLED(CONFIG_HAVE_BLOCK_DEVICE)) {
> > struct blk_desc *desc = dev_get_uclass_plat(dev);
> > @@ -843,6 +849,10 @@ static int blk_post_probe(struct udevice *dev)
> >
> > static int blk_part_post_probe(struct udevice *dev)
> > {
> > + if (CONFIG_IS_ENABLED(EFI_LOADER)) {
> > + if (efi_disk_create(dev))
> > + debug("*** efi_post_probe_device failed\n");
> > + }
> > /*
> > * TODO:
> > * If we call blk_creat_partitions() here, it would allow for
> > --
> > 2.33.0
> >
>
> Regards,
> Simon
Re: [PATCH 1/9] cache: sifive: Fix -Wint-to-pointer-cast warning
On Wed, Sep 15, 2021 at 11:40 AM Leo Liang wrote: > > On Sun, Sep 12, 2021 at 11:15:08AM +0800, Bin Meng wrote: > > The following warning is seen in cache-sifive-ccache.c in a 32-bit build: > > > > warning: cast to pointer from integer of different size > > [-Wint-to-pointer-cast] > > > > Fix by casting it with uintptr_t. > > > > Signed-off-by: Bin Meng > > --- > > > > drivers/cache/cache-sifive-ccache.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > Reviewed-by: Leo Yu-Chi Liang Ping for apply?
Re: [PATCH v2] board: sifive: Fix a potential build warning in board_fdt_blob_setup()
On Fri, Sep 17, 2021 at 2:52 PM Rick Chen wrote:
>
> > From: Bin Meng
> > Sent: Saturday, September 11, 2021 10:31 PM
> > To: Zong Li ; Leo Yu-Chi Liang(梁育齊)
> > ; Rick Jian-Zhi Chen(陳建志) ;
> > u-boot@lists.denx.de
> > Subject: [PATCH v2] board: sifive: Fix a potential build warning in
> > board_fdt_blob_setup()
> >
> > Commit 47d73ba4f4a4 ("board: sifive: overwrite board_fdt_blob_setup in
> > u-boot proper") added a board-specific implementation of
> > board_fdt_blob_setup() which takes a pointer as the return value, but it
> > does not return anything if CONFIG_OF_SEPARATE is not enabled. This will
> > cause a build warning seen when testing booting S-mode U-Boot directly from
> > QEMU, per the instructions in [1]:
> >
> > board/sifive/unleashed/unleashed.c: In function ‘board_fdt_blob_setup’:
> > board/sifive/unleashed/unleashed.c:125:1: warning: control reaches end of
> > non-void function [-Wreturn-type]
> >
> > Return &_end as the default case.
> >
> > [1]
> > https://qemu.readthedocs.io/en/latest/system/riscv/sifive_u.html#running-u-boot
> >
> > Signed-off-by: Bin Meng
> >
> > ---
> >
> > Changes in v2:
> > - Correct the commit title
> >
> > board/sifive/unleashed/unleashed.c | 4 ++--
> > board/sifive/unmatched/unmatched.c | 4 ++--
> > 2 files changed, 4 insertions(+), 4 deletions(-)
>
> Reviewed-by: Rick Chen
Ping for apply?
Re: [RFC 07/22] dm: blk: add UCLASS_PARTITION
Heinrich,
On Fri, Oct 08, 2021 at 10:23:52AM +0200, Heinrich Schuchardt wrote:
>
>
> On 10/8/21 02:51, AKASHI Takahiro wrote:
> > On Mon, Oct 04, 2021 at 12:27:59PM +0900, AKASHI Takahiro wrote:
> > > On Fri, Oct 01, 2021 at 11:30:37AM +0200, Heinrich Schuchardt wrote:
> > > >
> > > >
> > > > On 10/1/21 07:01, AKASHI Takahiro wrote:
> > > > > UCLASS_PARTITION device will be created as a child node of
> > > > > UCLASS_BLK device.
> > > > >
> > > > > Signed-off-by: AKASHI Takahiro
> > > > > ---
> > > > >drivers/block/blk-uclass.c | 111
> > > > > +
> > > > >include/blk.h | 9 +++
> > > > >include/dm/uclass-id.h | 1 +
> > > > >3 files changed, 121 insertions(+)
> > > > >
> > > > > diff --git a/drivers/block/blk-uclass.c b/drivers/block/blk-uclass.c
> > > > > index 83682dcc181a..dd7f3c0fe31e 100644
> > > > > --- a/drivers/block/blk-uclass.c
> > > > > +++ b/drivers/block/blk-uclass.c
> > > > > @@ -12,6 +12,7 @@
> > > > >#include
> > > > >#include
> > > > >#include
> > > > > +#include
> > > > >#include
> > > > >#include
> > > > >#include
> > > > > @@ -695,6 +696,44 @@ int blk_unbind_all(int if_type)
> > > > > return 0;
> > > > >}
> > > > >
> > > > > +int blk_create_partitions(struct udevice *parent)
> > > > > +{
> > > > > + int part, count;
> > > > > + struct blk_desc *desc = dev_get_uclass_plat(parent);
> > > > > + struct disk_partition info;
> > > > > + struct disk_part *part_data;
> > > > > + char devname[32];
> > > > > + struct udevice *dev;
> > > > > + int ret;
> > > > > +
> > > > > + if (!CONFIG_IS_ENABLED(PARTITIONS) ||
> > > > > + !CONFIG_IS_ENABLED(HAVE_BLOCK_DEVICE))
> > > > > + return 0;
> > > > > +
> > > > > + /* Add devices for each partition */
> > > > > + for (count = 0, part = 1; part <= MAX_SEARCH_PARTITIONS;
> > > > > part++) {
> > > > > + if (part_get_info(desc, part, &info))
> > > > > + continue;
> > > > > + snprintf(devname, sizeof(devname), "%s:%d",
> > > > > parent->name,
> > > > > + part);
> > > > > +
> > > > > + ret = device_bind_driver(parent, "blk_partition",
> > > > > + strdup(devname), &dev);
> > > > > + if (ret)
> > > > > + return ret;
> > > > > +
> > > > > + part_data = dev_get_uclass_plat(dev);
> > > > > + part_data->partnum = part;
> > > > > + part_data->gpt_part_info = info;
> > > > > + count++;
> > > > > +
> > > > > + device_probe(dev);
> > > > > + }
> > > > > + debug("%s: %d partitions found in %s\n", __func__, count,
> > > > > parent->name);
> > > > > +
> > > > > + return 0;
> > > > > +}
> > > > > +
> > > > >static int blk_post_probe(struct udevice *dev)
> > > > >{
> > > > > if (IS_ENABLED(CONFIG_PARTITIONS) &&
> > > > > @@ -713,3 +752,75 @@ UCLASS_DRIVER(blk) = {
> > > > > .post_probe = blk_post_probe,
> > > > > .per_device_plat_auto = sizeof(struct blk_desc),
> > > > >};
> > > > > +
> > > > > +static ulong blk_part_read(struct udevice *dev, lbaint_t start,
> > > > > +lbaint_t blkcnt, void *buffer)
> > > > > +{
> > > > > + struct udevice *parent;
> > > > > + struct disk_part *part;
> > > > > + const struct blk_ops *ops;
> > > > > +
> > > > > + parent = dev_get_parent(dev);
> > > >
> > > > What device type will the parent have if it is a eMMC hardware
> > > > partition?
> > > >
> > > > > + ops = blk_get_ops(parent);
> > > > > + if (!ops->read)
> > > > > + return -ENOSYS;
> > > > > +
> > > > > + part = dev_get_uclass_plat(dev);
> > > >
> > > > You should check that we do not access the block device past the
> > > > partition end:
> > >
> > > Yes, I will fix all of checks.
> > >
> > > > struct blk_desc *desc = dev_get_uclass_plat(parent);
> > > > if ((start + blkcnt) * desc->blksz < part->gpt_part_info.blksz)
> > > > return -EFAULT.
> > > >
> > > > > + start += part->gpt_part_info.start;
> >
> > A better solution is:
> > if (start >= part->gpt_part_info.size)
> > return 0;
> >
> > if ((start + blkcnt) > part->gpt_part_info.size)
> > blkcnt = part->gpt_part_info.size - start;
> > start += part->gpt_part_info.start;
> > instead of returning -EFAULT.
> > (note that start and blkcnt are in "block".)
>
> What is your motivation to support an illegal access?
>
> We will implement the EFI_BLOCK_IO_PROTOCOL based on this function. The
> ReadBlocks() and WriteBlocks() services must return
> EFI_INVALID_PARAMETER if the read request contains LBAs that are not
> valid.
I interpreted that 'LBA' was the third parameter to ReadBlocks API,
and that if the starting block is out of partition region, we should
return a
Re: [RFC 07/22] block: ide: call device_probe() after scanning
On Sun, Oct 10, 2021 at 08:14:13AM -0600, Simon Glass wrote:
> On Thu, 30 Sept 2021 at 23:03, AKASHI Takahiro
> wrote:
> >
> > Every time an ide bus/port is scanned and a new device is detected,
> > we want to call device_probe() as it will give us a chance to run additional
> > post-processings for some purposes.
> >
> > In particular, support for creating partitions on a device will be added.
> >
> > Signed-off-by: AKASHI Takahiro
> > ---
> > drivers/block/ide.c | 6 ++
> > 1 file changed, 6 insertions(+)
> >
>
> Reviewed-by: Simon Glass
>
> I'm starting to wonder if you can create a function that does the
> probe and unbind? Something in the blk interface, perhaps? It would
> reduce the duplicated code and provide a standard way of bringing up a
> new device.
That is exactly what Ilias suggested but I'm a bit declined to do :)
Common 'scanning' code looks like:
blk_create_devicef(... , &dev);
desc = dev_get_uclass_data(dev);
initialize some members in desc as well as device-specific info --- (A)
(now dev can be accessible.)
ret = device_probe(dev);
if (ret) {
de-initialize *dev* --- (B)
device_unbind()
}
Basically (B) is supposed to undo (A) which may or may not exist,
depending on types of block devices.
So I'm not 100% sure that a combination of device_probe() and device_unbind()
will fit to all the device types.
(The only cases that I have noticed are fsl_sata.c and sata_sil.c. Both
have their own xxx_unbind_device(), but they simply call device_remove() and
device_unbind(), though. So no worry?)
-Takahiro Akashi
> Perhaps blk_device_complete() ?
>
> Regards,
> Simon
Re: [RFC 06/22] sata: call device_probe() after scanning
On Sun, Oct 10, 2021 at 08:14:12AM -0600, Simon Glass wrote:
> Hi Takahiro,
>
> On Thu, 30 Sept 2021 at 23:03, AKASHI Takahiro
> wrote:
> >
> > Every time a sata bus/port is scanned and a new device is detected,
> > we want to call device_probe() as it will give us a chance to run additional
> > post-processings for some purposes.
> >
> > In particular, support for creating partitions on a device will be added.
> >
> > Signed-off-by: AKASHI Takahiro
> > ---
> > drivers/ata/dwc_ahsata.c | 10 ++
> > drivers/ata/fsl_sata.c | 11 +++
> > drivers/ata/sata_mv.c| 9 +
> > drivers/ata/sata_sil.c | 12
> > 4 files changed, 42 insertions(+)
> >
> > diff --git a/drivers/ata/dwc_ahsata.c b/drivers/ata/dwc_ahsata.c
> > index 6d42548087b3..6a51c70d1170 100644
> > --- a/drivers/ata/dwc_ahsata.c
> > +++ b/drivers/ata/dwc_ahsata.c
> > @@ -1026,6 +1026,16 @@ int dwc_ahsata_scan(struct udevice *dev)
> > return ret;
> > }
> >
> > + ret = device_probe(bdev);
> > + if (ret < 0) {
> > + debug("Can't probe\n");
> > + /* TODO: undo create */
> > +
> > + device_unbind(bdev);
>
> It looks like this one has the wrong variable name.
Ah, it was a copy-and-paste thing :)
I will fix it.
Thanks,
-Takahiro Akashi
> [..]
>
> Regards,
> Simon
Re: [RFC 03/22] mmc: call device_probe() after scanning
On Sun, Oct 10, 2021 at 08:14:09AM -0600, Simon Glass wrote:
> Hi Takahiro,
>
> On Thu, 30 Sept 2021 at 23:03, AKASHI Takahiro
> wrote:
> >
> > Every time a mmc bus/port is scanned and a new device is detected,
> > we want to call device_probe() as it will give us a chance to run additional
> > post-processings for some purposes.
> >
> > In particular, support for creating partitions on a device will be added.
> >
> > Signed-off-by: AKASHI Takahiro
> > ---
> > drivers/mmc/mmc-uclass.c | 7 +++
> > 1 file changed, 7 insertions(+)
> >
> > diff --git a/drivers/mmc/mmc-uclass.c b/drivers/mmc/mmc-uclass.c
> > index 3ee92d03ca23..07b5c1736439 100644
> > --- a/drivers/mmc/mmc-uclass.c
> > +++ b/drivers/mmc/mmc-uclass.c
> > @@ -418,6 +418,13 @@ int mmc_bind(struct udevice *dev, struct mmc *mmc,
> > const struct mmc_config *cfg)
> > bdesc->part_type = cfg->part_type;
> > mmc->dev = dev;
> > mmc->user_speed_mode = MMC_MODES_END;
> > +
> > + ret = device_probe(dev);
> > + if (ret) {
> > + device_unbind(dev);
> > + return ret;
> > + }
>
> We cannot probe a device within a bind() method. Can this be moved to
> mmc_blk_probe(), perhaps?
Ok, I will fix it.
Thanks,
-Takahiro Akashi
> Regards,
> Simon
Re: [PATCH v4 01/11] Revert "Revert "mkeficapsule: Remove dtb related options""
On Fri, Oct 08, 2021 at 10:11:59PM +0300, Ilias Apalodimas wrote:
> Hi Simon,
>
> On Fri, Oct 08, 2021 at 09:38:11AM -0600, Simon Glass wrote:
> > Hi Takahiro,
> >
> > On Thu, 7 Oct 2021 at 00:24, AKASHI Takahiro
> > wrote:
> > >
> > > This reverts commit d428e81266a59974ade74c1ba019af39f23304ab.
> > > We have agreed with removing dtb-related stuff from mkeficapsule
> > > command even if the commit 47a25e81d35c ("Revert "efi_capsule: Move
> > > signature from DTB to .rodata"") was applied.
> >
> > Can you please explain why this is being removed? How is the public
> > key to be communicated?
>
> Via the script that Akashi-san is adding in this patch series.
Yeah, please see my patch#4 for fdtsig.sh.
The reason why I think we should remove the feature from mkeficapsule
command is partly because we can do the same task by using *existing*
fdt tools and partly because having two totally-independent functionality
(one for a capsule binary and one for fdt) in a single tool seems confusing.
> >
> > Please can you also copy me on future related patches? (thank for you
> > for the private email alerting me)
> >
> > What does "We have agreed" mean?
>
> This has nothing to do with the public key. This is only changing the tool
> we used to include the key in the DTB. It is the same patch I mentioned you
> should never have reverted in the first place on your pull request, but since
> that happened very late in the release cycle we said we would re-apply it
> after
> the release.
Indeed.
-Takahiro Akashi
> >
> > >
> > > Signed-off-by: AKASHI Takahiro
> > > ---
> > > tools/mkeficapsule.c | 229 ++-
> > > 1 file changed, 7 insertions(+), 222 deletions(-)
> > >
> >
> > Regards,
> > Simon
>
> It was my patch to begin with so I am not really sure my reviewed tag has
> any value, but FWIW
>
> Reviewed-by: Ilias Apalodimas
Re: [PATCH 2/2] sunxi: fix non working console on uart2
On Sat, 9 Oct 2021 14:18:59 +0200
Angelo Dureghello wrote:
Hi Angelo,
can you please mention the H3 in the subject line, so it's more obvious
that's it's only about one SoC?
> Fix non working console on uart2, that seems releated to both
> Allwinner H2+ and H3.
>
> Signed-off-by: Angelo Dureghello
> ---
> arch/arm/include/asm/arch-sunxi/gpio.h | 1 +
> arch/arm/mach-sunxi/board.c| 10 +-
> 2 files changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/include/asm/arch-sunxi/gpio.h
> b/arch/arm/include/asm/arch-sunxi/gpio.h
> index 2969a530ae..21fcfd5638 100644
> --- a/arch/arm/include/asm/arch-sunxi/gpio.h
> +++ b/arch/arm/include/asm/arch-sunxi/gpio.h
> @@ -150,6 +150,7 @@ enum sunxi_gpio_number {
> #define SUN7I_GPA_GMAC 5
> #define SUN6I_GPA_SDC2 5
> #define SUN6I_GPA_SDC3 4
> +#define SUN8I_GPA_UART2 2
> #define SUN8I_H3_GPA_UART0 2
>
> #define SUN4I_GPB_PWM2
> diff --git a/arch/arm/mach-sunxi/board.c b/arch/arm/mach-sunxi/board.c
> index adb63e93e7..623da744e2 100644
> --- a/arch/arm/mach-sunxi/board.c
> +++ b/arch/arm/mach-sunxi/board.c
> @@ -145,10 +145,18 @@ static int gpio_init(void)
> sunxi_gpio_set_cfgpin(SUNXI_GPG(3), SUN5I_GPG_UART1);
> sunxi_gpio_set_cfgpin(SUNXI_GPG(4), SUN5I_GPG_UART1);
> sunxi_gpio_set_pull(SUNXI_GPG(4), SUNXI_GPIO_PULL_UP);
> -#elif CONFIG_CONS_INDEX == 3 && defined(CONFIG_MACH_SUN8I)
> +#elif CONFIG_CONS_INDEX == 3 && defined(CONFIG_MACH_SUN8I) && \
> + !defined(CONFIG_MACH_SUN8I_H2_PLUS) && \
> + !defined(CONFIG_MACH_SUN8I_H3)
> sunxi_gpio_set_cfgpin(SUNXI_GPB(0), SUN8I_GPB_UART2);
> sunxi_gpio_set_cfgpin(SUNXI_GPB(1), SUN8I_GPB_UART2);
> sunxi_gpio_set_pull(SUNXI_GPB(1), SUNXI_GPIO_PULL_UP);
> +#elif CONFIG_CONS_INDEX == 3 && defined(CONFIG_MACH_SUN8I) && \
CONFIG_MACH_SUN8I is redundant if you use CONFIG_MACH_SUN8I_H3 below,
so drop the former. And if you rearrange this section to come before
the existing one above, the #elif will save you changes there.
Also please drop CONFIG_MACH_SUN8I_H2_PLUS.
But the actual gist of this patch (UART2 is on portA) is indeed
correct.
Cheers,
Andre
> + (defined(CONFIG_MACH_SUN8I_H2_PLUS) || \
> + defined(CONFIG_MACH_SUN8I_H3))
> + sunxi_gpio_set_cfgpin(SUNXI_GPA(0), SUN8I_GPA_UART2);
> + sunxi_gpio_set_cfgpin(SUNXI_GPA(1), SUN8I_GPA_UART2);
> + sunxi_gpio_set_pull(SUNXI_GPA(1), SUNXI_GPIO_PULL_UP);
> #elif CONFIG_CONS_INDEX == 5 && defined(CONFIG_MACH_SUN8I)
> sunxi_gpio_set_cfgpin(SUNXI_GPL(2), SUN8I_GPL_R_UART);
> sunxi_gpio_set_cfgpin(SUNXI_GPL(3), SUN8I_GPL_R_UART);
[PATCH] loads: Block writes into LMB reserved areas of U-Boot
From: Marek Vasut
The loads srec loading may overwrite piece of U-Boot accidentally.
Prevent that by using LMB to detect whether upcoming write would
overwrite piece of reserved U-Boot code, and if that is the case,
abort the srec loading.
Signed-off-by: Marek Vasut
Cc: Simon Glass
Cc: Tom Rini
---
cmd/load.c | 12
1 file changed, 12 insertions(+)
diff --git a/cmd/load.c b/cmd/load.c
index 249ebd4ae0..7e4a552d90 100644
--- a/cmd/load.c
+++ b/cmd/load.c
@@ -16,6 +16,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -137,6 +138,7 @@ static int do_load_serial(struct cmd_tbl *cmdtp, int flag,
int argc,
static ulong load_serial(long offset)
{
+ struct lmb lmb;
charrecord[SREC_MAXRECLEN + 1]; /* buffer for one S-Record
*/
charbinbuf[SREC_MAXBINLEN]; /* buffer for binary data
*/
int binlen; /* no. of data bytes in S-Rec.
*/
@@ -147,6 +149,9 @@ static ulong load_serial(long offset)
ulong start_addr = ~0;
ulong end_addr = 0;
int line_count = 0;
+ long ret;
+
+ lmb_init_and_reserve(&lmb, gd->bd, (void *)gd->fdt_blob);
while (read_record(record, SREC_MAXRECLEN + 1) >= 0) {
type = srec_decode(record, &binlen, &addr, binbuf);
@@ -172,7 +177,14 @@ static ulong load_serial(long offset)
} else
#endif
{
+ ret = lmb_reserve(&lmb, store_addr, binlen);
+ if (ret) {
+ printf("\nCannot overwrite reserved area
(%08lx..%08lx)\n",
+ store_addr, store_addr + binlen);
+ return ret;
+ }
memcpy((char *)(store_addr), binbuf, binlen);
+ lmb_free(&lmb, store_addr, binlen);
}
if ((store_addr) < start_addr)
start_addr = store_addr;
--
2.33.0
[PATCH 1/2] arm64: Add missing GD_FLG_SKIP_RELOC handling
From: Marek Vasut In case U-Boot enters relocation with GD_FLG_SKIP_RELOC, skip the relocation. The code still has to set up new_gd pointer and new stack pointer. Signed-off-by: Marek Vasut Cc: Simon Glass Cc: Tom Rini --- arch/arm/lib/crt0_64.S | 4 lib/asm-offsets.c | 3 +++ 2 files changed, 7 insertions(+) diff --git a/arch/arm/lib/crt0_64.S b/arch/arm/lib/crt0_64.S index 680e674fa3..28c8356aee 100644 --- a/arch/arm/lib/crt0_64.S +++ b/arch/arm/lib/crt0_64.S @@ -104,6 +104,10 @@ ENTRY(_main) bic sp, x0, #0xf/* 16-byte alignment for ABI compliance */ ldr x18, [x18, #GD_NEW_GD] /* x18 <- gd->new_gd */ + /* Skip relocation in case gd->gd_flags & GD_FLG_SKIP_RELOC */ + ldr x0, [x18, #GD_FLAGS]/* x0 <- gd->flags */ + tbnzx0, 11, relocation_return /* GD_FLG_SKIP_RELOC is bit 11 */ + adr lr, relocation_return #if CONFIG_POSITION_INDEPENDENT /* Add in link-vs-runtime offset */ diff --git a/lib/asm-offsets.c b/lib/asm-offsets.c index c691066332..0808cd4b0c 100644 --- a/lib/asm-offsets.c +++ b/lib/asm-offsets.c @@ -29,6 +29,9 @@ int main(void) DEFINE(GD_SIZE, sizeof(struct global_data)); DEFINE(GD_BD, offsetof(struct global_data, bd)); + + DEFINE(GD_FLAGS, offsetof(struct global_data, flags)); + #if CONFIG_VAL(SYS_MALLOC_F_LEN) DEFINE(GD_MALLOC_BASE, offsetof(struct global_data, malloc_base)); #endif -- 2.33.0
[PATCH 2/2] arm64: lmb: Reserve U-Boot separately if relocation is disabled
From: Marek Vasut
In case U-Boot starts with GD_FLG_SKIP_RELOC, the U-Boot code is
not relocated, however the stack and heap is at the end of DRAM
after relocation. Reserve a LMB area for the non-relocated U-Boot
code so it won't be overwritten.
Signed-off-by: Marek Vasut
Cc: Simon Glass
Cc: Tom Rini
---
arch/arm/lib/stack.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/arch/arm/lib/stack.c b/arch/arm/lib/stack.c
index 656084c7e5..d2e2715ecf 100644
--- a/arch/arm/lib/stack.c
+++ b/arch/arm/lib/stack.c
@@ -14,6 +14,7 @@
#include
#include
#include
+#include
DECLARE_GLOBAL_DATA_PTR;
@@ -46,4 +47,12 @@ static ulong get_sp(void)
void arch_lmb_reserve(struct lmb *lmb)
{
arch_lmb_reserve_generic(lmb, get_sp(), gd->ram_top, 16384);
+
+#ifdef CONFIG_ARM
+ if (gd->flags & GD_FLG_SKIP_RELOC) {
+ lmb_reserve(lmb, (phys_addr_t)__image_copy_start,
+ (phys_addr_t)__image_copy_end -
+ (phys_addr_t)__image_copy_start);
+ }
+#endif
}
--
2.33.0
[PATCH] efi_loader: Handle GD_FLG_SKIP_RELOC
From: Marek Vasut
In case U-Boot starts with GD_FLG_SKIP_RELOC, the efi loader
relocation code breaks down because it assumes gd->relocaddr
points to relocated U-Boot code, which is not the case. Add
special case for handling GD_FLG_SKIP_RELOC, which uses the
__image_copy_start instead of gd->relocaddr for efi loader
code relocation source address.
Signed-off-by: Marek Vasut
Cc: Heinrich Schuchardt
Cc: Simon Glass
Cc: Tom Rini
---
common/board_r.c | 7 ++-
lib/efi_loader/efi_runtime.c | 19 ---
2 files changed, 22 insertions(+), 4 deletions(-)
diff --git a/common/board_r.c b/common/board_r.c
index 31a59c585a..3e95123f95 100644
--- a/common/board_r.c
+++ b/common/board_r.c
@@ -178,7 +178,12 @@ static int initr_reloc_global_data(void)
*/
efi_save_gd();
- efi_runtime_relocate(gd->relocaddr, NULL);
+#ifdef CONFIG_ARM
+ if (gd->flags & GD_FLG_SKIP_RELOC)
+ efi_runtime_relocate((ulong)__image_copy_start, NULL);
+ else
+#endif
+ efi_runtime_relocate(gd->relocaddr, NULL);
#endif
return 0;
diff --git a/lib/efi_loader/efi_runtime.c b/lib/efi_loader/efi_runtime.c
index 93a695fc27..876ca09106 100644
--- a/lib/efi_loader/efi_runtime.c
+++ b/lib/efi_loader/efi_runtime.c
@@ -17,6 +17,8 @@
#include
#include
+#include
+
/* For manual relocation support */
DECLARE_GLOBAL_DATA_PTR;
@@ -629,13 +631,23 @@ out:
return ret;
}
+static unsigned long efi_get_reloc_start(void)
+{
+#ifdef CONFIG_ARM
+ if (gd->flags & GD_FLG_SKIP_RELOC)
+ return (unsigned long)__image_copy_start;
+ else
+#endif
+ return gd->relocaddr;
+}
+
static __efi_runtime void efi_relocate_runtime_table(ulong offset)
{
ulong patchoff;
void **pos;
/* Relocate the runtime services pointers */
- patchoff = offset - gd->relocaddr;
+ patchoff = offset - efi_get_reloc_start();
for (pos = (void **)&efi_runtime_services.get_time;
pos <= (void **)&efi_runtime_services.query_variable_info; ++pos) {
if (*pos)
@@ -681,7 +693,7 @@ void efi_runtime_relocate(ulong offset, struct efi_mem_desc
*map)
ulong *p;
ulong newaddr;
- p = (void*)((ulong)rel->offset - base) + gd->relocaddr;
+ p = (void*)((ulong)rel->offset - base) + efi_get_reloc_start();
/*
* The runtime services table is updated in
@@ -852,7 +864,8 @@ static efi_status_t EFIAPI efi_set_virtual_address_map(
map = (void*)virtmap + (descriptor_size * i);
if (map->type == EFI_RUNTIME_SERVICES_CODE) {
ulong new_offset = map->virtual_start -
- map->physical_start + gd->relocaddr;
+ map->physical_start +
+ efi_get_reloc_start();
efi_relocate_runtime_table(new_offset);
efi_runtime_relocate(new_offset, map);
--
2.33.0
[PATCH] board_f: Copy GD to new GD even if relocation disabled
Even if U-Boot has relocation disabled via GD_FLG_SKIP_RELOC , the
relocated stage of U-Boot still picks GD from new_gd location. The
U-Boot itself is not relocated, but GD might be, so copy the GD to
new GD location even if relocation is disabled.
Signed-off-by: Marek Vasut
Cc: Simon Glass
Cc: Tom Rini
---
common/board_f.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/common/board_f.c b/common/board_f.c
index 3dc0eaa59c..2161a7411d 100644
--- a/common/board_f.c
+++ b/common/board_f.c
@@ -674,6 +674,7 @@ static int reloc_bloblist(void)
static int setup_reloc(void)
{
if (gd->flags & GD_FLG_SKIP_RELOC) {
+ memcpy(gd->new_gd, (char *)gd, sizeof(gd_t));
debug("Skipping relocation due to flag\n");
return 0;
}
--
2.33.0
Re: Please pull u-boot-video
On Sat, Oct 09, 2021 at 11:29:23PM +0200, Anatolij Gustschin wrote: > Hi Tom, > > please pull new video patches. > > gitlab CI: > https://source.denx.de/u-boot/custodians/u-boot-video/-/pipelines/9433 > > Thanks, > Anatolij > > The following changes since commit 94e922c76a0e1fcdead3359e340f53fd0709a963: > > Merge branch '2021-10-08-image-cleanups' (2021-10-08 16:02:55 -0400) > > are available in the Git repository at: > > https://source.denx.de/u-boot/custodians/u-boot-video.git > tags/video-20211009 > > for you to fetch changes up to 79c05335a9c101f0b54f2f378d0b08c9b765e1a3: > > video: move MXS to Kconfig (2021-10-09 19:50:03 +0200) > Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
[PATCH] tools: Stop re-defining -std= when building tools
While we intentionally set -std=gnu11 for building host tools, and have for quite some time, we never dropped -std=gnu99 from tools/Makefile. This resulted in passing -std=gnu11 ... -std=gnu99 when building, and gnu99 would win. This in turn would result now in warnings such as: tools/mkeficapsule.c:25:15: warning: redefinition of typedef 'u32' is a C11 feature [-Wtypedef-redefinition] typedef __u32 u32; ^ Signed-off-by: Tom Rini --- tools/Makefile | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tools/Makefile b/tools/Makefile index 999fd4653166..b45219e2c30c 100644 --- a/tools/Makefile +++ b/tools/Makefile @@ -295,8 +295,7 @@ HOST_EXTRACFLAGS += -include $(srctree)/include/compiler.h \ -I$(srctree)/tools \ -DUSE_HOSTCC \ -D__KERNEL_STRICT_NAMES \ - -D_GNU_SOURCE \ - -std=gnu99 + -D_GNU_SOURCE __build: $(LOGO-y) -- 2.25.1
uboot FIT signature difference
Hi, i try to create a upstream uboot binary for use with rockchip rk3568 (bananapi r2 pro). Currently i'm on preparation phase as i don't have the hardware yet (so i cannot test it). First thanks to everybody upstreamed support for this SOC in uboot and linux. i found a compiled uboot.img with this signature: FIT Image with ATF/OP-TEE/U-Boot my created itb (make u-boot.itb) has this signature: FIT image for U-Boot with bl31 (TF-A) can anybody give me a hint how to create same signature as above? or where i have to look... i guess the uboot.img is created using this source: https://github.com/rockchip-linux/u-boot as far as i see the itb gets generated from the dtb OBJCOPY u-boot-nodtb.bin ./"arch/arm/mach-rockchip/make_fit_atf.py" \ arch/arm/dts/rk3568-evb.dtb > u-boot.its RELOC u-boot-nodtb.bin MKIMAGE u-boot.itb op-tee seems to be a lib only in rockchip repo, not upstream https://github.com/rockchip-linux/u-boot/tree/next-dev/lib/optee_clientApi seems to be some kind of secure boot, idk if this is needed to boot the device i wonder why uboot differentiate between ATF and BL31...imho BL31 is part of ATF, do i need all parts of ATF (at least BL2) to get same signature? i have some rk3568_ddr_MHz_v1.08.bin which can be BL2 (did not found any signature and Rockchip ATF source is afaik not yet released, at least not for rk35xx which is new). "my" source is this: https://github.com/frank-w/u-boot/tree/2021-10-bpi-r2-pro regards Frank
Re: [PATCH v2 0/1] sunxi: Add support for FriendlyARM NanoPi R1S H5
On Sun, 10 Oct 2021 21:36:56 +0800 Chukun Pan wrote: > Hi Andre, > > > Please don't include not-yet-merged parts in here. Make a verbatim copy > > of the version in Linus' tree. > > Okay, I remove the not-yet-merged parts in dts. > > > who is reading the MAC address out of the EEPROM, and how > > does it land in the kernel (if it lands there)? > > There is a rtl8153 usb network card on this machine, and it will > randomly generate a mac address when it bootup. I think the mac > address on the eeprom is for this rtl8153 usb network card. Well, fair enough, but how does this MAC address end up in the USB MAC? I guess this misses some code, but IIUC Chen-Yu pointed you to the solution already in the Linux thread. > > So why is CONFIG_MACPWR missing from the defconfig? > > This is my mistake, I'm very sorry about that. Many thanks for the quick turnaround, that looks alright now. I will push it as part of the sunxi PR for v2022.01. > > Thanks, > Chukun > > Chukun Pan (1): > sunxi: Add support for FriendlyARM NanoPi R1S H5 Reviewed-by: Andre Przywara Cheers, Andre > > arch/arm/dts/Makefile| 1 + > arch/arm/dts/sun50i-h5-nanopi-r1s-h5.dts | 195 +++ > board/sunxi/MAINTAINERS | 5 + > configs/nanopi_r1s_h5_defconfig | 14 ++ > 4 files changed, 215 insertions(+) > create mode 100644 arch/arm/dts/sun50i-h5-nanopi-r1s-h5.dts > create mode 100644 configs/nanopi_r1s_h5_defconfig >
[PATCH] drivers: ddr: lc_common_dimm_params.c : Fix Divison by zero issue
Adds check for memory clock variable before calculating caslat_actual.
Set mclk_ps to slowest DIMM supported if mclk_ps is found zero.
Signed-off-by: Maninder Singh
---
drivers/ddr/fsl/lc_common_dimm_params.c | 10 --
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/ddr/fsl/lc_common_dimm_params.c
b/drivers/ddr/fsl/lc_common_dimm_params.c
index d299d763db..d738ae3a7c 100644
--- a/drivers/ddr/fsl/lc_common_dimm_params.c
+++ b/drivers/ddr/fsl/lc_common_dimm_params.c
@@ -1,7 +1,7 @@
// SPDX-License-Identifier: GPL-2.0
/*
* Copyright 2008-2016 Freescale Semiconductor, Inc.
- * Copyright 2017-2018 NXP Semiconductor
+ * Copyright 2017-2021 NXP Semiconductor
*/
#include
@@ -23,7 +23,7 @@ compute_cas_latency(const unsigned int ctrl_num,
unsigned int caslat_actual;
unsigned int retry = 16;
unsigned int tmp = ~0;
- const unsigned int mclk_ps = get_memory_clk_period_ps(ctrl_num);
+ unsigned int mclk_ps = get_memory_clk_period_ps(ctrl_num);
#ifdef CONFIG_SYS_FSL_DDR3
const unsigned int taamax = 2;
#else
@@ -37,6 +37,12 @@ compute_cas_latency(const unsigned int ctrl_num,
}
common_caslat = tmp;
+ if (!mclk_ps) {
+ printf("DDR clock (MCLK cycle was 0 ps), So setting it to
slowest DIMM(s) (tCKmin %u ps).\n",
+ outpdimm->tckmin_x_ps);
+ mclk_ps = outpdimm->tckmin_x_ps;
+ }
+
/* validate if the memory clk is in the range of dimms */
if (mclk_ps < outpdimm->tckmin_x_ps) {
printf("DDR clock (MCLK cycle %u ps) is faster than "
--
2.17.1
[PATCH v2 1/1] sunxi: Add support for FriendlyARM NanoPi R1S H5
This adds support for the NanoPi R1S H5 board.
Allwinner H5 SoC
512MB DDR3 RAM
10/100/1000M Ethernet x 2
RTL8189ETV WiFi 802.11b/g/n
USB 2.0 host port (A)
MicroSD Slot
Reset button
Serial Debug Port
WAN - LAN - SYS LED
The dts file is taken from Linux 5.14 tag.
Signed-off-by: Chukun Pan
---
Changes for v2:
- Remove not-yet-merged parts in dts
- Add missing CONFIG_MACPWR option
arch/arm/dts/Makefile| 1 +
arch/arm/dts/sun50i-h5-nanopi-r1s-h5.dts | 195 +++
board/sunxi/MAINTAINERS | 5 +
configs/nanopi_r1s_h5_defconfig | 14 ++
4 files changed, 215 insertions(+)
create mode 100644 arch/arm/dts/sun50i-h5-nanopi-r1s-h5.dts
create mode 100644 configs/nanopi_r1s_h5_defconfig
diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
index 9e44817a40..e278480c1c 100644
--- a/arch/arm/dts/Makefile
+++ b/arch/arm/dts/Makefile
@@ -632,6 +632,7 @@ dtb-$(CONFIG_MACH_SUN50I_H5) += \
sun50i-h5-libretech-all-h5-cc.dtb \
sun50i-h5-nanopi-neo2.dtb \
sun50i-h5-nanopi-neo-plus2.dtb \
+ sun50i-h5-nanopi-r1s-h5.dtb \
sun50i-h5-orangepi-zero-plus.dtb \
sun50i-h5-orangepi-pc2.dtb \
sun50i-h5-orangepi-prime.dtb \
diff --git a/arch/arm/dts/sun50i-h5-nanopi-r1s-h5.dts
b/arch/arm/dts/sun50i-h5-nanopi-r1s-h5.dts
new file mode 100644
index 00..55bcdf8d1a
--- /dev/null
+++ b/arch/arm/dts/sun50i-h5-nanopi-r1s-h5.dts
@@ -0,0 +1,195 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Copyright (C) 2021 Chukun Pan
+ *
+ * Based on sun50i-h5-nanopi-neo-plus2.dts, which is:
+ * Copyright (C) 2017 Antony Antony
+ * Copyright (C) 2016 ARM Ltd.
+ */
+
+/dts-v1/;
+#include "sun50i-h5.dtsi"
+#include "sun50i-h5-cpu-opp.dtsi"
+
+#include
+#include
+#include
+
+/ {
+ model = "FriendlyARM NanoPi R1S H5";
+ compatible = "friendlyarm,nanopi-r1s-h5", "allwinner,sun50i-h5";
+
+ aliases {
+ ethernet0 = &emac;
+ ethernet1 = &rtl8189etv;
+ serial0 = &uart0;
+ };
+
+ chosen {
+ stdout-path = "serial0:115200n8";
+ };
+
+ leds {
+ compatible = "gpio-leds";
+
+ led-0 {
+ function = LED_FUNCTION_LAN;
+ color = ;
+ gpios = <&pio 0 9 GPIO_ACTIVE_HIGH>;
+ };
+
+ led-1 {
+ function = LED_FUNCTION_STATUS;
+ color = ;
+ gpios = <&pio 0 10 GPIO_ACTIVE_HIGH>;
+ linux,default-trigger = "heartbeat";
+ };
+
+ led-2 {
+ function = LED_FUNCTION_WAN;
+ color = ;
+ gpios = <&pio 6 11 GPIO_ACTIVE_HIGH>;
+ };
+ };
+
+ r-gpio-keys {
+ compatible = "gpio-keys";
+
+ reset {
+ label = "reset";
+ linux,code = ;
+ gpios = <&r_pio 0 3 GPIO_ACTIVE_LOW>;
+ };
+ };
+
+ reg_gmac_3v3: gmac-3v3 {
+ compatible = "regulator-fixed";
+ regulator-name = "gmac-3v3";
+ regulator-min-microvolt = <330>;
+ regulator-max-microvolt = <330>;
+ startup-delay-us = <10>;
+ enable-active-high;
+ gpio = <&pio 3 6 GPIO_ACTIVE_HIGH>;
+ };
+
+ reg_vcc3v3: vcc3v3 {
+ compatible = "regulator-fixed";
+ regulator-name = "vcc3v3";
+ regulator-min-microvolt = <330>;
+ regulator-max-microvolt = <330>;
+ };
+
+ reg_usb0_vbus: usb0-vbus {
+ compatible = "regulator-fixed";
+ regulator-name = "usb0-vbus";
+ regulator-min-microvolt = <500>;
+ regulator-max-microvolt = <500>;
+ enable-active-high;
+ gpio = <&r_pio 0 2 GPIO_ACTIVE_HIGH>; /* PL2 */
+ status = "okay";
+ };
+
+ vdd_cpux: gpio-regulator {
+ compatible = "regulator-gpio";
+ regulator-name = "vdd-cpux";
+ regulator-type = "voltage";
+ regulator-boot-on;
+ regulator-always-on;
+ regulator-min-microvolt = <110>;
+ regulator-max-microvolt = <130>;
+ regulator-ramp-delay = <50>; /* 4ms */
+ gpios = <&r_pio 0 6 GPIO_ACTIVE_HIGH>;
+ gpios-states = <0x1>;
+ states = <110 0x0>, <130 0x1>;
+ };
+
+ wifi_pwrseq: wifi_pwrseq {
+ compatible = "mmc-pwrseq-simple";
+ reset-gpios = <&r_pio 0 7 GPIO_ACTIVE_LOW>; /* PL7 */
+ post-power-on-delay-ms = <200>;
+ };
+};
+
+&cpu0 {
+ cpu-supply = <&vdd_cpux>;
+};
+
+&ehci1 {
+ status = "okay
[PATCH v2 0/1] sunxi: Add support for FriendlyARM NanoPi R1S H5
Hi Andre, > Please don't include not-yet-merged parts in here. Make a verbatim copy > of the version in Linus' tree. Okay, I remove the not-yet-merged parts in dts. > who is reading the MAC address out of the EEPROM, and how > does it land in the kernel (if it lands there)? There is a rtl8153 usb network card on this machine, and it will randomly generate a mac address when it bootup. I think the mac address on the eeprom is for this rtl8153 usb network card. > So why is CONFIG_MACPWR missing from the defconfig? This is my mistake, I'm very sorry about that. Thanks, Chukun Chukun Pan (1): sunxi: Add support for FriendlyARM NanoPi R1S H5 arch/arm/dts/Makefile| 1 + arch/arm/dts/sun50i-h5-nanopi-r1s-h5.dts | 195 +++ board/sunxi/MAINTAINERS | 5 + configs/nanopi_r1s_h5_defconfig | 14 ++ 4 files changed, 215 insertions(+) create mode 100644 arch/arm/dts/sun50i-h5-nanopi-r1s-h5.dts create mode 100644 configs/nanopi_r1s_h5_defconfig -- 2.17.1
Re: [PATCH 1/2] sunxi: add H2+ config option
Hi Andre,
On Sun, Oct 10, 2021 at 1:48 AM Andre Przywara wrote:
>
> On Sat, 9 Oct 2021 14:18:58 +0200
> Angelo Dureghello wrote:
>
> Hi Angelo,
>
> > Add H2+ Kconfig oiption to display proper cpu at boot time,
> > and for other future uses, if needed, to differentiate against H3.
> >
> > The patch does not change any board defconfig at this stage,
> > so that nothing is broken.
> >
> > Tested this CONFIG_MACH_SUN8I_H2_PLUS to work properly on
> > banana pi m2 zero.
>
> But why? What does that fix except the hardcoded CPU output line? I
> only see a lot of churn and future issues (forgetting to add
> MACH_SUN8I_H2_PLUS). As far as we know the two SoCs are very close, and
> are almost indistinguishable by software, that's why we treat them the
> same.
> So is there a good reason for differentiating the two?
> If you are *really* desperate about that one "CPU: ..." line, have a
> look at https://linux-sunxi.org/H3#Variants and see if you can make this
> decision at runtime, check A31 and A31s for an example.
This "desperate" sounds a bit ugly :) I can live without of course :) .
I faced the uart2 console not working issue (fixed in 2/2), and initially
thought it was a H2+ / H3 difference, but it was not.
Anyway, i finally thought that a differentiation may be useful If not,
just trow the patch in the bin.
In the case you may still be interested differentiating the two just to
print cpu model, or any future need, an alternative safer proposal may be
something like:
config MACH_SUN8I_H2_PLUS
select MACH_SUN8I_H3
>
> Cheers,
> Andre
>
cheers,
angelo
> > Signed-off-by: Angelo Dureghello
> > ---
> > arch/arm/cpu/armv7/sunxi/Makefile | 1 +
> > arch/arm/cpu/armv7/sunxi/psci.c | 2 ++
> > arch/arm/cpu/armv7/sunxi/tzpc.c | 2 +-
> > arch/arm/dts/Makefile | 5 +
> > arch/arm/include/asm/arch-sunxi/cpu_sun4i.h | 4 ++--
> > arch/arm/mach-sunxi/Kconfig | 9 +
> > arch/arm/mach-sunxi/board.c | 3 ++-
> > arch/arm/mach-sunxi/cpu_info.c | 6 --
> > arch/arm/mach-sunxi/dram_sunxi_dw.c | 4 ++--
> > drivers/clk/sunxi/Makefile | 1 +
> > drivers/video/sunxi/sunxi_de2.c | 2 +-
> > 11 files changed, 30 insertions(+), 9 deletions(-)
> >
> > diff --git a/arch/arm/cpu/armv7/sunxi/Makefile
> > b/arch/arm/cpu/armv7/sunxi/Makefile
> > index 1d40d6a18d..5fdfe45401 100644
> > --- a/arch/arm/cpu/armv7/sunxi/Makefile
> > +++ b/arch/arm/cpu/armv7/sunxi/Makefile
> > @@ -8,6 +8,7 @@
> > obj-y+= timer.o
> >
> > obj-$(CONFIG_MACH_SUN6I) += tzpc.o
> > +obj-$(CONFIG_MACH_SUN8I_H2_PLUS) += tzpc.o
> > obj-$(CONFIG_MACH_SUN8I_H3) += tzpc.o
> >
> > ifndef CONFIG_SPL_BUILD
> > diff --git a/arch/arm/cpu/armv7/sunxi/psci.c
> > b/arch/arm/cpu/armv7/sunxi/psci.c
> > index 1ac50f558a..bcdbe888bc 100644
> > --- a/arch/arm/cpu/armv7/sunxi/psci.c
> > +++ b/arch/arm/cpu/armv7/sunxi/psci.c
> > @@ -79,6 +79,7 @@ static void __secure __mdelay(u32 ms)
> > static void __secure clamp_release(u32 __maybe_unused *clamp)
> > {
> > #if defined(CONFIG_MACH_SUN6I) || defined(CONFIG_MACH_SUN7I) || \
> > + defined(CONFIG_MACH_SUN8I_H2_PLUS) || \
> > defined(CONFIG_MACH_SUN8I_H3) || \
> > defined(CONFIG_MACH_SUN8I_R40)
> > u32 tmp = 0x1ff;
> > @@ -94,6 +95,7 @@ static void __secure clamp_release(u32 __maybe_unused
> > *clamp)
> > static void __secure clamp_set(u32 __maybe_unused *clamp)
> > {
> > #if defined(CONFIG_MACH_SUN6I) || defined(CONFIG_MACH_SUN7I) || \
> > + defined(CONFIG_MACH_SUN8I_H2_PLUS) || \
> > defined(CONFIG_MACH_SUN8I_H3) || \
> > defined(CONFIG_MACH_SUN8I_R40)
> > writel(0xff, clamp);
> > diff --git a/arch/arm/cpu/armv7/sunxi/tzpc.c
> > b/arch/arm/cpu/armv7/sunxi/tzpc.c
> > index 0c86a21a3f..44640509ff 100644
> > --- a/arch/arm/cpu/armv7/sunxi/tzpc.c
> > +++ b/arch/arm/cpu/armv7/sunxi/tzpc.c
> > @@ -17,7 +17,7 @@ void tzpc_init(void)
> > writel(SUN6I_TZPC_DECPORT0_RTC, &tzpc->decport0_set);
> > #endif
> >
> > -#ifdef CONFIG_MACH_SUN8I_H3
> > +#if defined(CONFIG_MACH_SUN8I_H2_PLUS) || defined(CONFIG_MACH_SUN8I_H3)
> > /* Enable non-secure access to all peripherals */
> > writel(SUN8I_H3_TZPC_DECPORT0_ALL, &tzpc->decport0_set);
> > writel(SUN8I_H3_TZPC_DECPORT1_ALL, &tzpc->decport1_set);
> > diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
> > index b8a382d153..2e58815769 100644
> > --- a/arch/arm/dts/Makefile
> > +++ b/arch/arm/dts/Makefile
> > @@ -598,6 +598,11 @@ dtb-$(CONFIG_MACH_SUN8I_A83T) += \
> > sun8i-a83t-bananapi-m3.dtb \
> > sun8i-a83t-cubietruck-plus.dtb \
> > sun8i-a83t-tbs-a711.dtb
> > +dtb-$(CONFIG_MACH_SUN8I_H2_PLUS) += \
> > + sun8i-h2-plus-bananapi-m2-zero.dtb \
> > + sun8i-h2-plus-libretech-all-h3-cc.dtb \
> > + sun8i-h2-plus-orangepi-r1.dtb \
> > + sun8i-h2-plus-orangepi-zero.dtb
> > dtb-$(CONFIG_MACH_SUN8I_H3) += \
> > sun8
Re: [RFC 11/22] efi_loader: disk: use udevice instead of blk_desc
On Thu, 30 Sept 2021 at 23:04, AKASHI Takahiro
wrote:
>
> In most of all usages, we can avoid accessing blk_desc which is eventually
> an internal data structure to be hided outside block device drivers.
>
> Signed-off-by: AKASHI Takahiro
> ---
> lib/efi_loader/efi_disk.c | 25 -
> 1 file changed, 12 insertions(+), 13 deletions(-)
>
Reviewed-by: Simon Glass
But needs updating to use media_read() or similar
> diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
> index 988907ecb910..dfa6f898d586 100644
> --- a/lib/efi_loader/efi_disk.c
> +++ b/lib/efi_loader/efi_disk.c
> @@ -45,7 +45,7 @@ struct efi_disk_obj {
> unsigned int part;
> struct efi_simple_file_system_protocol *volume;
> lbaint_t offset;
> - struct blk_desc *desc;
> + struct udevice *dev; /* TODO: move it to efi_object */
> };
>
> /**
> @@ -80,14 +80,12 @@ static efi_status_t efi_disk_rw_blocks(struct
> efi_block_io *this,
> void *buffer, enum efi_disk_direction direction)
> {
> struct efi_disk_obj *diskobj;
> - struct blk_desc *desc;
> int blksz;
> int blocks;
> unsigned long n;
>
> diskobj = container_of(this, struct efi_disk_obj, ops);
> - desc = (struct blk_desc *) diskobj->desc;
> - blksz = desc->blksz;
> + blksz = diskobj->media.block_size;
> blocks = buffer_size / blksz;
> lba += diskobj->offset;
>
> @@ -99,9 +97,9 @@ static efi_status_t efi_disk_rw_blocks(struct efi_block_io
> *this,
> return EFI_BAD_BUFFER_SIZE;
>
> if (direction == EFI_DISK_READ)
> - n = blk_dread(desc, lba, blocks, buffer);
> + n = blk_read(diskobj->dev, lba, blocks, buffer);
> else
> - n = blk_dwrite(desc, lba, blocks, buffer);
> + n = blk_write(diskobj->dev, lba, blocks, buffer);
>
> /* We don't do interrupts, so check for timers cooperatively */
> efi_timer_check();
> @@ -443,7 +441,6 @@ static efi_status_t efi_disk_add_dev(
> diskobj->ops = block_io_disk_template;
> diskobj->ifname = if_typename;
> diskobj->dev_index = dev_index;
> - diskobj->desc = desc;
>
> /* Fill in EFI IO Media info (for read/write callbacks) */
> diskobj->media.removable_media = desc->removable;
> @@ -647,20 +644,22 @@ bool efi_disk_is_system_part(efi_handle_t handle)
> {
> struct efi_handler *handler;
> struct efi_disk_obj *diskobj;
> - struct disk_partition info;
> + struct udevice *dev;
> + struct disk_part *part;
> efi_status_t ret;
> - int r;
>
> /* check if this is a block device */
> ret = efi_search_protocol(handle, &efi_block_io_guid, &handler);
> if (ret != EFI_SUCCESS)
> return false;
>
> + /* find a partition udevice */
> diskobj = container_of(handle, struct efi_disk_obj, header);
> -
> - r = part_get_info(diskobj->desc, diskobj->part, &info);
> - if (r)
> + dev = diskobj->dev;
> + if (!dev || dev->driver->id != UCLASS_PARTITION)
> return false;
>
> - return !!(info.bootable & PART_EFI_SYSTEM_PARTITION);
> + part = dev_get_uclass_plat(dev);
> +
> + return !!(part->gpt_part_info.bootable & PART_EFI_SYSTEM_PARTITION);
> }
> --
> 2.33.0
>
Re: [RFC 13/22] efi_loader: remove !CONFIG_BLK code from efi_disk
On Thu, 30 Sept 2021 at 23:04, AKASHI Takahiro wrote: > > The change in this patch will probably have been covered by other guy's patch. > > Signed-off-by: AKASHI Takahiro > --- > lib/efi_loader/efi_disk.c | 49 --- > 1 file changed, 49 deletions(-) > Reviewed-by: Simon Glass
Re: [RFC 10/22] dm: blk: add read/write interfaces with udevice
Hi Takahiro,
On Thu, 30 Sept 2021 at 23:03, AKASHI Takahiro
wrote:
>
> In include/blk.h, Simon suggested:
> ===>
> /*
> * These functions should take struct udevice instead of struct blk_desc,
> * but this is convenient for migration to driver model. Add a 'd' prefix
> * to the function operations, so that blk_read(), etc. can be reserved for
> * functions with the correct arguments.
> */
> unsigned long blk_dread(struct blk_desc *block_dev, lbaint_t start,
> lbaint_t blkcnt, void *buffer);
> unsigned long blk_dwrite(struct blk_desc *block_dev, lbaint_t start,
> lbaint_t blkcnt, const void *buffer);
> unsigned long blk_derase(struct blk_desc *block_dev, lbaint_t start,
> lbaint_t blkcnt);
> <===
>
> So new interfaces are provided with this patch.
>
> They are expected to be used everywhere in U-Boot at the end. The exceptions
> are block device drivers, partition drivers and efi_disk which should know
> details of blk_desc structure.
>
> Signed-off-by: AKASHI Takahiro
> ---
> drivers/block/blk-uclass.c | 91 ++
> include/blk.h | 6 +++
> 2 files changed, 97 insertions(+)
>
> diff --git a/drivers/block/blk-uclass.c b/drivers/block/blk-uclass.c
> index 6ba11a8fa7f7..8fbec8779e1e 100644
> --- a/drivers/block/blk-uclass.c
> +++ b/drivers/block/blk-uclass.c
> @@ -482,6 +482,97 @@ unsigned long blk_derase(struct blk_desc *block_dev,
> lbaint_t start,
> return ops->erase(dev, start, blkcnt);
> }
>
> +static struct blk_desc *dev_get_blk(struct udevice *dev)
> +{
> + struct blk_desc *block_dev;
> +
> + switch (device_get_uclass_id(dev)) {
> + case UCLASS_BLK:
> + block_dev = dev_get_uclass_plat(dev);
> + break;
> + case UCLASS_PARTITION:
> + block_dev = dev_get_uclass_plat(dev_get_parent(dev));
> + break;
> + default:
> + block_dev = NULL;
> + break;
> + }
> +
> + return block_dev;
> +}
> +
> +unsigned long blk_read(struct udevice *dev, lbaint_t start,
> + lbaint_t blkcnt, void *buffer)
> +{
> + struct blk_desc *block_dev;
> + const struct blk_ops *ops;
> + struct disk_part *part;
> + lbaint_t start_in_disk;
> + ulong blks_read;
> +
> + block_dev = dev_get_blk(dev);
> + if (!block_dev)
> + return -ENOSYS;
IMO blk_read() should take a block device. That is how all other DM
APIs work. I think it is too confusing to use the parent device here.
So how about changing these functions to take a blk device, then
adding new ones for what you want here, e.g.
int media_read(struct udevice *dev...
{
struct udevice *blk;
blk = dev_get_blk(dev);
if (!blk)
return -ENOSYS;
ret = blk_read(blk, ...
}
Also can we use blk instead of block_dev?
> +
> + ops = blk_get_ops(dev);
> + if (!ops->read)
> + return -ENOSYS;
> +
> + start_in_disk = start;
> + if (device_get_uclass_id(dev) == UCLASS_PARTITION) {
> + part = dev_get_uclass_plat(dev);
> + start_in_disk += part->gpt_part_info.start;
> + }
> +
> + if (blkcache_read(block_dev->if_type, block_dev->devnum,
> + start_in_disk, blkcnt, block_dev->blksz, buffer))
> + return blkcnt;
> + blks_read = ops->read(dev, start, blkcnt, buffer);
> + if (blks_read == blkcnt)
> + blkcache_fill(block_dev->if_type, block_dev->devnum,
> + start_in_disk, blkcnt, block_dev->blksz,
> buffer);
> +
> + return blks_read;
> +}
> +
> +unsigned long blk_write(struct udevice *dev, lbaint_t start,
> + lbaint_t blkcnt, const void *buffer)
> +{
> + struct blk_desc *block_dev;
> + const struct blk_ops *ops;
> +
> + block_dev = dev_get_blk(dev);
> + if (!block_dev)
> + return -ENOSYS;
> +
> + ops = blk_get_ops(dev);
> + if (!ops->write)
> + return -ENOSYS;
> +
> + blkcache_invalidate(block_dev->if_type, block_dev->devnum);
> +
> + return ops->write(dev, start, blkcnt, buffer);
> +}
> +
> +unsigned long blk_erase(struct udevice *dev, lbaint_t start,
> + lbaint_t blkcnt)
> +{
> + struct blk_desc *block_dev;
> + const struct blk_ops *ops;
> +
> + block_dev = dev_get_blk(dev);
> + if (!block_dev)
> + return -ENOSYS;
> +
> + ops = blk_get_ops(dev);
> + if (!ops->erase)
> + return -ENOSYS;
> +
> + blkcache_invalidate(block_dev->if_type, block_dev->devnum);
> +
> + return ops->erase(dev, start, blkcnt);
> +}
> +
> int blk_get_from_parent(struct udevice *parent, struct udevice **devp)
> {
> struct udevice *dev;
> diff --git a/include/blk.h b/include/blk.h
> index 3d883eb1d
Re: [RFC 07/22] block: ide: call device_probe() after scanning
On Thu, 30 Sept 2021 at 23:03, AKASHI Takahiro wrote: > > Every time an ide bus/port is scanned and a new device is detected, > we want to call device_probe() as it will give us a chance to run additional > post-processings for some purposes. > > In particular, support for creating partitions on a device will be added. > > Signed-off-by: AKASHI Takahiro > --- > drivers/block/ide.c | 6 ++ > 1 file changed, 6 insertions(+) > Reviewed-by: Simon Glass I'm starting to wonder if you can create a function that does the probe and unbind? Something in the blk interface, perhaps? It would reduce the duplicated code and provide a standard way of bringing up a new device. Perhaps blk_device_complete() ? Regards, Simon
Re: [RFC 08/22] dm: blk: add a device-probe hook for scanning disk partitions
On Thu, 30 Sept 2021 at 23:03, AKASHI Takahiro
wrote:
>
> Now that all the block device drivers have enable a probe hook, we will
> call blk_create_partitions() to enumerate all the partitions and create
> associated udevices when a block device is detected.
>
> Signed-off-by: AKASHI Takahiro
> ---
> drivers/block/blk-uclass.c | 15 +++
> 1 file changed, 15 insertions(+)
>
Reviewed-by: Simon Glass
But I don't see blk_create_partitions() in this patch.
> diff --git a/drivers/block/blk-uclass.c b/drivers/block/blk-uclass.c
> index dd7f3c0fe31e..6ba11a8fa7f7 100644
> --- a/drivers/block/blk-uclass.c
> +++ b/drivers/block/blk-uclass.c
> @@ -741,11 +741,25 @@ static int blk_post_probe(struct udevice *dev)
> struct blk_desc *desc = dev_get_uclass_plat(dev);
>
> part_init(desc);
> +
> + if (desc->part_type != PART_TYPE_UNKNOWN &&
> + blk_create_partitions(dev))
> + debug("*** creating partitions failed\n");
> }
>
> return 0;
> }
>
> +static int blk_part_post_probe(struct udevice *dev)
> +{
> + /*
> +* TODO:
> +* If we call blk_creat_partitions() here, it would allow for
> +* "partitions in a partition".
> +*/
Would that be useful?
> + return 0;
> +}
> +
> UCLASS_DRIVER(blk) = {
> .id = UCLASS_BLK,
> .name = "blk",
> @@ -821,6 +835,7 @@ U_BOOT_DRIVER(blk_partition) = {
>
> UCLASS_DRIVER(partition) = {
> .id = UCLASS_PARTITION,
> + .post_probe = blk_part_post_probe,
> .per_device_plat_auto = sizeof(struct disk_part),
> .name = "partition",
> };
> --
> 2.33.0
>
Re: [RFC 07/22] dm: blk: add UCLASS_PARTITION
Hi Takahiro,
On Thu, 30 Sept 2021 at 23:03, AKASHI Takahiro
wrote:
>
> UCLASS_PARTITION device will be created as a child node of
> UCLASS_BLK device.
>
> Signed-off-by: AKASHI Takahiro
> ---
> drivers/block/blk-uclass.c | 111 +
> include/blk.h | 9 +++
> include/dm/uclass-id.h | 1 +
> 3 files changed, 121 insertions(+)
>
> diff --git a/drivers/block/blk-uclass.c b/drivers/block/blk-uclass.c
> index 83682dcc181a..dd7f3c0fe31e 100644
> --- a/drivers/block/blk-uclass.c
> +++ b/drivers/block/blk-uclass.c
> @@ -12,6 +12,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -695,6 +696,44 @@ int blk_unbind_all(int if_type)
> return 0;
> }
>
> +int blk_create_partitions(struct udevice *parent)
> +{
> + int part, count;
> + struct blk_desc *desc = dev_get_uclass_plat(parent);
> + struct disk_partition info;
> + struct disk_part *part_data;
> + char devname[32];
> + struct udevice *dev;
> + int ret;
> +
> + if (!CONFIG_IS_ENABLED(PARTITIONS) ||
> + !CONFIG_IS_ENABLED(HAVE_BLOCK_DEVICE))
> + return 0;
> +
> + /* Add devices for each partition */
> + for (count = 0, part = 1; part <= MAX_SEARCH_PARTITIONS; part++) {
> + if (part_get_info(desc, part, &info))
> + continue;
> + snprintf(devname, sizeof(devname), "%s:%d", parent->name,
> +part);
> +
> + ret = device_bind_driver(parent, "blk_partition",
> +strdup(devname), &dev);
> + if (ret)
> + return ret;
> +
> + part_data = dev_get_uclass_plat(dev);
> + part_data->partnum = part;
> + part_data->gpt_part_info = info;
> + count++;
> +
> + device_probe(dev);
> + }
> + debug("%s: %d partitions found in %s\n", __func__, count,
> parent->name);
log_debug() and drop __func__
> +
> + return 0;
> +}
> +
> static int blk_post_probe(struct udevice *dev)
> {
> if (IS_ENABLED(CONFIG_PARTITIONS) &&
> @@ -713,3 +752,75 @@ UCLASS_DRIVER(blk) = {
> .post_probe = blk_post_probe,
> .per_device_plat_auto = sizeof(struct blk_desc),
> };
> +
> +static ulong blk_part_read(struct udevice *dev, lbaint_t start,
> + lbaint_t blkcnt, void *buffer)
> +{
part_blk_read() so that it is clear that this takes a UCLASS_PARTITION
device, not a UCLASS_BLK
> + struct udevice *parent;
> + struct disk_part *part;
> + const struct blk_ops *ops;
> +
> + parent = dev_get_parent(dev);
> + ops = blk_get_ops(parent);
> + if (!ops->read)
> + return -ENOSYS;
> +
> + part = dev_get_uclass_plat(dev);
> + start += part->gpt_part_info.start;
> +
> + return ops->read(parent, start, blkcnt, buffer);
> +}
> +
> +static ulong blk_part_write(struct udevice *dev, lbaint_t start,
> + lbaint_t blkcnt, const void *buffer)
> +{
> + struct udevice *parent;
> + struct disk_part *part;
> + const struct blk_ops *ops;
> +
> + parent = dev_get_parent(dev);
> + ops = blk_get_ops(parent);
> + if (!ops->write)
> + return -ENOSYS;
> +
> + part = dev_get_uclass_plat(dev);
> + start += part->gpt_part_info.start;
> +
> + return ops->write(parent, start, blkcnt, buffer);
> +}
> +
> +static ulong blk_part_erase(struct udevice *dev, lbaint_t start,
> + lbaint_t blkcnt)
> +{
> + struct udevice *parent;
> + struct disk_part *part;
> + const struct blk_ops *ops;
> +
> + parent = dev_get_parent(dev);
> + ops = blk_get_ops(parent);
> + if (!ops->erase)
> + return -ENOSYS;
> +
> + part = dev_get_uclass_plat(dev);
> + start += part->gpt_part_info.start;
> +
> + return ops->erase(parent, start, blkcnt);
> +}
> +
> +static const struct blk_ops blk_part_ops = {
> + .read = blk_part_read,
> + .write = blk_part_write,
> + .erase = blk_part_erase,
> +};
> +
> +U_BOOT_DRIVER(blk_partition) = {
> + .name = "blk_partition",
> + .id = UCLASS_PARTITION,
> + .ops= &blk_part_ops,
> +};
Can you put this all in a separate part-uclass.c file? This too:
> +
> +UCLASS_DRIVER(partition) = {
> + .id = UCLASS_PARTITION,
> + .per_device_plat_auto = sizeof(struct disk_part),
> + .name = "partition",
> +};
> diff --git a/include/blk.h b/include/blk.h
> index 19bab081c2cd..3d883eb1db64 100644
> --- a/include/blk.h
> +++ b/include/blk.h
> @@ -366,6 +366,15 @@ int blk_create_devicef(struct udevice *parent, const
> char *drv_name,
>const char *name, int if_type, int devnum, int
Re: [RFC 01/22] part: call part_init() in blk_get_device_by_str() only for MMC
On Thu, 30 Sept 2021 at 23:02, AKASHI Takahiro wrote: > > In blk_get_device_by_str(), the comment says: "Updates the partition table > for the specified hw partition." > Since hw partition is supported only on MMC, it makes no sense to do so > for other devices. > > Signed-off-by: AKASHI Takahiro > --- > disk/part.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) Reviewed-by: Simon Glass
Re: [RFC 19/22] dm: blk: call efi's device-removal hook
Hi Takahiro, On Thu, 30 Sept 2021 at 23:05, AKASHI Takahiro wrote: > > Adding the callback function, efi_disk_delete(), in block devices's > pre_remove hook will allows for automatically deleting efi_disk objects > per block device. > > This will eliminate any improper efi_disk objects which hold a link to > non-existing udevice structures when associated block devices are physically > un-plugged or udevices are once removed (and re-created) by executing commands > like "scsi rescan." > > Signed-off-by: AKASHI Takahiro > --- > drivers/block/blk-uclass.c | 22 ++ > 1 file changed, 22 insertions(+) Should use event mechanism as mentioned in previous patch. Regards, Simon
Re: [RFC 18/22] efi_loader: efi_disk: a helper function to delete efi_disk objects
On Thu, 30 Sept 2021 at 23:04, AKASHI Takahiro wrote: > > This function is expected to be called, in particular from dm's pre_remove > hook, when associated block devices no longer exist. > > Signed-off-by: AKASHI Takahiro > --- > include/efi_loader.h | 2 ++ > lib/efi_loader/efi_disk.c | 54 +++ > 2 files changed, 56 insertions(+) Reviewed-by: Simon Glass Please add full function comments.
Re: [RFC 20/22] efi_driver: align with efi_disk-dm integration
Hi Takahiro,
On Thu, 30 Sept 2021 at 23:05, AKASHI Takahiro
wrote:
>
Can you please add a commit message as I am not sure what this patch is doing.
> Signed-off-by: AKASHI Takahiro
> ---
> lib/efi_driver/efi_block_device.c | 6 ++
> lib/efi_loader/efi_device_path.c | 29 +
> lib/efi_loader/efi_disk.c | 12 +++-
> 3 files changed, 46 insertions(+), 1 deletion(-)
>
> diff --git a/lib/efi_driver/efi_block_device.c
> b/lib/efi_driver/efi_block_device.c
> index 0937e3595a43..b6afa939e1d1 100644
> --- a/lib/efi_driver/efi_block_device.c
> +++ b/lib/efi_driver/efi_block_device.c
> @@ -173,6 +173,12 @@ static int efi_bl_bind(efi_handle_t handle, void
> *interface)
> plat->handle = handle;
> plat->io = interface;
>
> + /*
> +* FIXME: necessary because we won't do almost nothing in
> +* efi_disk_create() when called from device_probe().
> +*/
> + bdev->efi_obj = handle;
> +
> ret = device_probe(bdev);
> if (ret)
> return ret;
> diff --git a/lib/efi_loader/efi_device_path.c
> b/lib/efi_loader/efi_device_path.c
> index cbdb466da41c..36c77bce9a05 100644
> --- a/lib/efi_loader/efi_device_path.c
> +++ b/lib/efi_loader/efi_device_path.c
> @@ -628,6 +628,35 @@ __maybe_unused static void *dp_fill(void *buf, struct
> udevice *dev)
> return &dp->vendor_data[1];
> }
> #endif
> +#ifdef CONFIG_EFI_LOADER
> + /*
> +* FIXME: conflicting with CONFIG_SANDBOX
In what way? If it is just a test we should be able to update the test
to avoid the conflict.
> +* This case is necessary to support efi_disk's created by
> +* efi_driver (and efi_driver_binding_protocol).
> +* TODO:
> +* The best way to work around here is to create efi_root as
> +* udevice and put all efi_driver objects under it.
> +*/
> + case UCLASS_ROOT: {
> + struct efi_device_path_vendor *dp;
> + struct blk_desc *desc = dev_get_uclass_plat(dev);
> + /* FIXME: guid_vendor used in selftest_block_device */
> + static efi_guid_t guid_vendor =
> + EFI_GUID(0xdbca4c98, 0x6cb0, 0x694d,
> + 0x08, 0x72, 0x81, 0x9c, 0x65, 0x0c, 0xb7,
> 0xb8);
> +
> +
> + dp_fill(buf, dev->parent);
> + dp = buf;
> + ++dp;
> + dp->dp.type = DEVICE_PATH_TYPE_HARDWARE_DEVICE;
> + dp->dp.sub_type = DEVICE_PATH_SUB_TYPE_VENDOR;
> + dp->dp.length = sizeof(*dp) + 1;
> + memcpy(&dp->guid, &guid_vendor, sizeof(efi_guid_t));
> + dp->vendor_data[0] = desc->devnum;
> + return &dp->vendor_data[1];
> + }
> +#endif
> #ifdef CONFIG_VIRTIO_BLK
> case UCLASS_VIRTIO: {
> struct efi_device_path_vendor *dp;
> diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
> index dfd06dd31e4a..e7cf1567929b 100644
> --- a/lib/efi_loader/efi_disk.c
> +++ b/lib/efi_loader/efi_disk.c
> @@ -562,11 +562,21 @@ static int efi_disk_create_part(struct udevice *dev)
> int efi_disk_create(struct udevice *dev)
> {
> enum uclass_id id;
> + struct blk_desc *desc;
>
> id = device_get_uclass_id(dev);
>
> - if (id == UCLASS_BLK)
> + if (id == UCLASS_BLK) {
> + /*
> +* avoid creating duplicated objects now that efi_driver
> +* has already created an efi_disk at this moment.
> +*/
> + desc = dev_get_uclass_plat(dev);
> + if (desc->if_type == IF_TYPE_EFI)
> + return 0;
> +
> return efi_disk_create_raw(dev);
> + }
>
> if (id == UCLASS_PARTITION)
> return efi_disk_create_part(dev);
> --
> 2.33.0
>
Regards,
Simon
Re: [RFC 12/22] dm: add a hidden link to efi object
Hi Takahiro,
On Thu, 30 Sept 2021 at 23:04, AKASHI Takahiro
wrote:
>
> This member field in udevice will be used to dereference from udevice
> to efi_object (or efi_handle).
>
> Signed-off-by: AKASHI Takahiro
> ---
> include/dm/device.h | 4
> 1 file changed, 4 insertions(+)
I think this should be generalised.
Can we add a simple API for attaching things to devices? Something like:
config DM_TAG
bool "Support tags attached to devices"
enum dm_tag_t {
DM_TAG_EFI = 0,
DM_TAG_COUNT,
};
ret = dev_tag_set_ptr(dev, DM_TAG_EFI, ptr);
void *ptr = dev_tag_get_ptr(dev, DM_TAG_EFI);
ulong val = dev_tag_get_val(dev, DM_TAG_EFI);
Under the hood I think for now we could have a simple list of tags for
all of DM:
struct dmtag_node {
struct list_head sibling;
struct udevice *dev;
enum dm_tag_t tag;
union {
void *ptr;
ulong val;
};
};
This can be useful in other situations, for example I think we need to
be able to send an event when a device is probed so that other devices
(with tags attached) can take action. But in any case, it makes the
API separate from the data structure, so aids refactoring later.
If we find that this is slow we can change the impl, but I doubt it
will matter fornow.
>
> diff --git a/include/dm/device.h b/include/dm/device.h
> index 0a9718a5b81a..33b09a836f06 100644
> --- a/include/dm/device.h
> +++ b/include/dm/device.h
> @@ -190,6 +190,10 @@ struct udevice {
> #if CONFIG_IS_ENABLED(DM_DMA)
> ulong dma_offset;
> #endif
> +#if CONFIG_IS_ENABLED(EFI_LOADER)
> + /* link to efi_object */
> + void *efi_obj;
> +#endif
> };
>
> /**
> --
> 2.33.0
>
Regards,
Simon
Re: [RFC 14/22] efi_loader: disk: a helper function to create efi_disk objects from udevice
On Thu, 30 Sept 2021 at 23:04, AKASHI Takahiro
wrote:
>
> Add efi_disk_create() function.
>
> Any UEFI handle created by efi_disk_create() can be treated as a efi_disk
> object, the udevice is either a UCLASS_BLK (a whole raw disk) or
> UCLASS_PARTITION (a disk partition).
>
> So this function is expected to be called every time such an udevice
> is detected and activated through a device model's "probe" interface.
>
> Signed-off-by: AKASHI Takahiro
> ---
> include/efi_loader.h | 2 +
> lib/efi_loader/efi_disk.c | 92 +++
> 2 files changed, 94 insertions(+)
>
Reviewed-by: Simon Glass
But some nits below.
Don't worry about !CONFIG_BLK - that code should be removed.
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index c440962fe522..751fde7fb153 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -517,6 +517,8 @@ efi_status_t EFIAPI efi_convert_pointer(efi_uintn_t
> debug_disposition,
> void efi_carve_out_dt_rsv(void *fdt);
> /* Called by bootefi to make console interface available */
> efi_status_t efi_console_register(void);
> +/* Called when a block devices has been probed */
> +int efi_disk_create(struct udevice *dev);
Please buck the trend in this file and add a full function comment. In
this case it needs to cover @dev and the return value and also explain
what the function does.
> /* Called by bootefi to make all disk storage accessible as EFI objects */
> efi_status_t efi_disk_register(void);
> /* Called by efi_init_obj_list() to install EFI_RNG_PROTOCOL */
> diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
> index cd5528046251..3fae40e034fb 100644
> --- a/lib/efi_loader/efi_disk.c
> +++ b/lib/efi_loader/efi_disk.c
> @@ -10,6 +10,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -484,6 +485,7 @@ error:
> return ret;
> }
>
> +#ifndef CONFIG_BLK
> /**
> * efi_disk_create_partitions() - create handles and protocols for partitions
> *
> @@ -531,6 +533,96 @@ int efi_disk_create_partitions(efi_handle_t parent,
> struct blk_desc *desc,
>
> return disks;
> }
> +#endif /* CONFIG_BLK */
> +
> +/*
> + * Create a handle for a whole raw disk
> + *
> + * @devuclass device
?? what type of device?
> + * @return 0 on success, -1 otherwise
> + */
> +static int efi_disk_create_raw(struct udevice *dev)
> +{
> + struct efi_disk_obj *disk;
> + struct blk_desc *desc;
> + const char *if_typename;
> + int diskid;
> + efi_status_t ret;
> +
> + desc = dev_get_uclass_plat(dev);
> + if_typename = blk_get_if_type_name(desc->if_type);
> + diskid = desc->devnum;
> +
> + ret = efi_disk_add_dev(NULL, NULL, if_typename, desc,
> + diskid, NULL, 0, &disk);
> + if (ret != EFI_SUCCESS) {
if (ret)
is much shorter and easier to read
> + log_err("Adding disk %s%d failed\n", if_typename, diskid);
> + return -1;
> + }
> + disk->dev = dev;
> + dev->efi_obj = &disk->header;
> +
> + return 0;
> +}
> +
> +/*
> + * Create a handle for a disk partition
> + *
> + * @devuclass device
??
> + * @return 0 on success, -1 otherwise
> + */
> +static int efi_disk_create_part(struct udevice *dev)
> +{
> + efi_handle_t parent;
> + struct blk_desc *desc;
> + const char *if_typename;
> + struct disk_part *part_data;
> + struct disk_partition *info;
> + unsigned int part;
> + int diskid;
> + struct efi_device_path *dp = NULL;
> + struct efi_disk_obj *disk;
> + efi_status_t ret;
> +
> + parent = dev->parent->efi_obj;
dev_get_parent(dev)
> + desc = dev_get_uclass_plat(dev->parent);
> + if_typename = blk_get_if_type_name(desc->if_type);
> + diskid = desc->devnum;
> +
> + part_data = dev_get_uclass_plat(dev);
> + part = part_data->partnum;
> + info = &part_data->gpt_part_info;
> +
> + /* TODO: should not use desc? */
> + dp = efi_dp_from_part(desc, 0);
> +
> + ret = efi_disk_add_dev(parent, dp, if_typename, desc, diskid,
> + info, part, &disk);
> + if (ret != EFI_SUCCESS) {
> + log_err("Adding partition %s%d:%x failed\n",
> + if_typename, diskid, part);
> + return -1;
> + }
> + disk->dev = dev;
> + dev->efi_obj = &disk->header;
> +
> + return 0;
> +}
> +
> +int efi_disk_create(struct udevice *dev)
> +{
> + enum uclass_id id;
> +
> + id = device_get_uclass_id(dev);
> +
> + if (id == UCLASS_BLK)
> + return efi_disk_create_raw(dev);
> +
> + if (id == UCLASS_PARTITION)
> + return efi_disk_create_part(dev);
> +
> + return -1;
> +}
>
> /**
> * efi_disk_register() - register block devices
> --
> 2.33.0
>
Regards,
Simon
Re: [RFC 21/22] efi_driver: cleanup after efi_disk-dm integration
On Thu, 30 Sept 2021 at 23:05, AKASHI Takahiro wrote: > > efi_driver-specific binding will be no longer needed now that efi_disk- > dm integration takes care of efi_driver case as well. > > Signed-off-by: AKASHI Takahiro > --- > lib/efi_driver/efi_block_device.c | 24 > 1 file changed, 24 deletions(-) > Reviewed-by: Simon Glass
Re: [RFC 14/22] dm: blk: call efi's device-probe hook
Hi Takahiro,
On Thu, 30 Sept 2021 at 23:04, AKASHI Takahiro
wrote:
>
> Adding this callback function, efi_disk_create() in block devices's
> post_probe hook will allows for automatically creating efi_disk objects
> per block device.
>
> This will end up not only eliminating efi_disk_register() called in UEFI
> initialization, but also enabling detections of new block devices even
> after the initialization.
>
> Signed-off-by: AKASHI Takahiro
> ---
> drivers/block/blk-uclass.c | 10 ++
> 1 file changed, 10 insertions(+)
This is where events come in. We need a way to notify things when
devices go through different stages, etc.
I am thinking of:
enum dm_event_t {
DMEVT_PROBE,
DMEVT_POST_PROBE,
...
};
struct dm_event {
enum dm_event_t type;
union {
// add data for different event types in here
} data;
}
int (*dm_handler_t)(void *ctx, struct dm_event *event);
int dm_register(enum dm_event_t evtype, dm_handler_t func, void *ctx)
int dm_notify(struct udevice *dev, enum dm_event_t type, void *data);
Then the code below becomes:
dm_notify(struct udevice *dev, DMEVT_POST_PROBE, NULL);
the implementation of which will call the handler.
If you like I could create an impl of the above as a separate patch
for discussion.
>
> diff --git a/drivers/block/blk-uclass.c b/drivers/block/blk-uclass.c
> index 8fbec8779e1e..ce45cf0a8768 100644
> --- a/drivers/block/blk-uclass.c
> +++ b/drivers/block/blk-uclass.c
> @@ -9,6 +9,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -827,6 +828,11 @@ int blk_create_partitions(struct udevice *parent)
>
> static int blk_post_probe(struct udevice *dev)
> {
> + if (CONFIG_IS_ENABLED(EFI_LOADER)) {
> + if (efi_disk_create(dev))
> + debug("*** efi_post_probe_device failed\n");
> + }
> +
> if (IS_ENABLED(CONFIG_PARTITIONS) &&
> IS_ENABLED(CONFIG_HAVE_BLOCK_DEVICE)) {
> struct blk_desc *desc = dev_get_uclass_plat(dev);
> @@ -843,6 +849,10 @@ static int blk_post_probe(struct udevice *dev)
>
> static int blk_part_post_probe(struct udevice *dev)
> {
> + if (CONFIG_IS_ENABLED(EFI_LOADER)) {
> + if (efi_disk_create(dev))
> + debug("*** efi_post_probe_device failed\n");
> + }
> /*
> * TODO:
> * If we call blk_creat_partitions() here, it would allow for
> --
> 2.33.0
>
Regards,
Simon
Re: [RFC 17/22] efi_loader: add efi_remove_handle()
On Thu, 30 Sept 2021 at 23:04, AKASHI Takahiro wrote: > > This function is a counterpart of efi_add_handle() and will be used > in order to remove an efi_disk object in a later patch. > > Signed-off-by: AKASHI Takahiro > --- > include/efi_loader.h | 2 ++ > lib/efi_loader/efi_boottime.c | 8 > 2 files changed, 10 insertions(+) Reviewed-by: Simon Glass
Re: [RFC 16/22] efi_loader: cleanup after efi_disk-dm integration
On Thu, 30 Sept 2021 at 23:04, AKASHI Takahiro wrote: > > efi_disk_register() will be no longer needed now that all efi_disks are > set to be created with device model thanks to efi_disk-dm integration. > > Signed-off-by: AKASHI Takahiro > --- > include/efi_loader.h | 2 - > lib/efi_loader/efi_disk.c | 102 - > lib/efi_loader/efi_setup.c | 5 -- > 3 files changed, 109 deletions(-) > Reviewed-by: Simon Glass
Re: [RFC 06/22] sata: call device_probe() after scanning
Hi Takahiro,
On Thu, 30 Sept 2021 at 23:03, AKASHI Takahiro
wrote:
>
> Every time a sata bus/port is scanned and a new device is detected,
> we want to call device_probe() as it will give us a chance to run additional
> post-processings for some purposes.
>
> In particular, support for creating partitions on a device will be added.
>
> Signed-off-by: AKASHI Takahiro
> ---
> drivers/ata/dwc_ahsata.c | 10 ++
> drivers/ata/fsl_sata.c | 11 +++
> drivers/ata/sata_mv.c| 9 +
> drivers/ata/sata_sil.c | 12
> 4 files changed, 42 insertions(+)
>
> diff --git a/drivers/ata/dwc_ahsata.c b/drivers/ata/dwc_ahsata.c
> index 6d42548087b3..6a51c70d1170 100644
> --- a/drivers/ata/dwc_ahsata.c
> +++ b/drivers/ata/dwc_ahsata.c
> @@ -1026,6 +1026,16 @@ int dwc_ahsata_scan(struct udevice *dev)
> return ret;
> }
>
> + ret = device_probe(bdev);
> + if (ret < 0) {
> + debug("Can't probe\n");
> + /* TODO: undo create */
> +
> + device_unbind(bdev);
It looks like this one has the wrong variable name.
[..]
Regards,
Simon
Re: [RFC 03/22] mmc: call device_probe() after scanning
Hi Takahiro,
On Thu, 30 Sept 2021 at 23:03, AKASHI Takahiro
wrote:
>
> Every time a mmc bus/port is scanned and a new device is detected,
> we want to call device_probe() as it will give us a chance to run additional
> post-processings for some purposes.
>
> In particular, support for creating partitions on a device will be added.
>
> Signed-off-by: AKASHI Takahiro
> ---
> drivers/mmc/mmc-uclass.c | 7 +++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/mmc/mmc-uclass.c b/drivers/mmc/mmc-uclass.c
> index 3ee92d03ca23..07b5c1736439 100644
> --- a/drivers/mmc/mmc-uclass.c
> +++ b/drivers/mmc/mmc-uclass.c
> @@ -418,6 +418,13 @@ int mmc_bind(struct udevice *dev, struct mmc *mmc, const
> struct mmc_config *cfg)
> bdesc->part_type = cfg->part_type;
> mmc->dev = dev;
> mmc->user_speed_mode = MMC_MODES_END;
> +
> + ret = device_probe(dev);
> + if (ret) {
> + device_unbind(dev);
> + return ret;
> + }
We cannot probe a device within a bind() method. Can this be moved to
mmc_blk_probe(), perhaps?
Regards,
Simon
Re: [RFC 05/22] nvme: call device_probe() after scanning
On Thu, 30 Sept 2021 at 23:03, AKASHI Takahiro wrote: > > Every time a nvme bus/port is scanned and a new device is detected, > we want to call device_probe() as it will give us a chance to run additional > post-processings for some purposes. > > In particular, support for creating partitions on a device will be added. > > Signed-off-by: AKASHI Takahiro > --- > drivers/nvme/nvme.c | 6 ++ > 1 file changed, 6 insertions(+) Reviewed-by: Simon Glass
Re: [RFC 03/22] usb: storage: call device_probe() after scanning
On Thu, 30 Sept 2021 at 23:03, AKASHI Takahiro wrote: > > Every time a usb bus/port is scanned and a new device is detected, > we want to call device_probe() as it will give us a chance to run additional > post-processings for some purposes. > > In particular, support for creating partitions on a device will be added. > > Signed-off-by: AKASHI Takahiro > --- > common/usb_storage.c | 6 ++ > 1 file changed, 6 insertions(+) Reviewed-by: Simon Glass
Re: [RFC 01/22] scsi: call device_probe() after scanning
On Thu, 30 Sept 2021 at 23:02, AKASHI Takahiro wrote: > > Every time a scsi bus/port is scanned and a new block device is detected, > we want to call device_probe() as it will give us a chance to run additional > post-processings for some purposes. > > In particular, support for creating partitions on a device will be added. > > Signed-off-by: AKASHI Takahiro > --- > drivers/scsi/scsi.c | 10 ++ > 1 file changed, 10 insertions(+) Reviewed-by: Simon Glass
Re: [RFC 00/22] efi_loader: more tightly integrate UEFI disks to device model
Hi Takahiro, On Thu, 30 Sept 2021 at 23:02, AKASHI Takahiro wrote: > > The purpose of this RPC is to reignite the discussion about how UEFI > subystem would best be integrated into U-Boot device model. > In the past, I poposed a couple of patch series, the latest one[1], > while Heinrich revealed his idea[2], and the approach taken here is > something between them, with a focus on block device handlings. > > # The code is a PoC and not well tested yet. > > Disks in UEFI world: > > In general in UEFI world, accessing to any device is performed through > a 'protocol' interface which are installed to (or associated with) the > device's > UEFI handle (or an opaque pointer to UEFI object data). Protocols are > implemented by either the UEFI system itself or UEFI drivers. > > For block IO's, it is a device which has EFI_BLOCK_IO_PROTOCOL (efi_disk > hereafter). Currently, every efi_disk may have one of two origins: > a.U-Boot's block devices or related partitions > (lib/efi_loader/efi_disk.c) > b.UEFI objects which are implemented as a block device by UEFI drivers. > (lib/efi_driver/efi_block_device.c) > > All the efi_diskss as (a) will be enumelated and created only once at UEFI > subsystem initialization (efi_disk_register()), which is triggered by > first executing one of UEFI-related U-Boot commands, like "bootefi", > "setenv -e" or "efidebug". > EFI_BLOCK_IO_PROTOCOL is implemented by UEFI system using blk_desc(->ops) > in the corresponding udevice(UCLASS_BLK). > > On the other hand, efi_disk as (b) will be created each time UEFI boot > services' connect_controller() is executed in UEFI app which, as a (device) > controller, gives the method to access the device's data, > ie. EFI_BLOCK_IO_PROTOCOL. > > >>> more details >>> > Internally, connect_controller() search for UEFI driver that can support > this controller/protocol, 'efi_block' driver(UCLASS_EFI) in this case, > then calls the driver's 'bind' interface, which eventually installs > the controller's EFI_BLOCK_IO_PROTOCOL to efi_disk object. > 'efi_block' driver also create a corresponding udevice(UCLASS_BLK) for > * creating additional partitions efi_disk's, and > * supporting a file system (EFI_SIMPLE_FILE_SYSTEM_PROTOCOL) on it. > <<< <<< > > Issues: > === > 1. While an efi_disk represents a device equally for either a whole disk >or a partition in UEFI world, the device model treats only a whole >disk as a real block device or udevice(UCLASS_BLK). > > 2. efi_disk holds and makes use of "blk_desc" data even though blk_desc >in plat_data is supposed to be private and not to be accessed outside >the device model. ># This issue, though, exists for all the implmenetation of U-Boot ># file systems as well. Yes, this was a migration convenience and we should be able to unpick it now. However we still have SPL_BLK so need to consider whether we can drop that. > > For efi_disk(a), > 3. A block device can be enumelated dynamically by 'scanning' a device bus >in U-Boot, but UEFI subsystem is not able to update efi_disks accordingly. >For examples, > => scsi rescan; efidebug devices > => usb start; efidebug devices ... (A) >(A) doesn't show any usb devices detected. > > => scsi rescan; efidebug boot add -b 0 TEST scsi 0:1 ... > => scsi rescan ... (B) > => bootefi bootmgr ... (C) >(C) may de-reference a bogus blk_desc pointer which has been freed by (B). >(Please note that "scsi rescan" removes all udevices/blk_desc and then > re-create them even if nothing is changed on a bus.) This is something your series will help with. > > For efi_disk(b), > 4. A controller (handle), combined with efi_block driver, has no >corresponding udevice as a parent of efi_disks in DM tree, unlike, say, >a scsi controller, even though it provides methods for block io perations. Can we just create a udevice? > 5. There is no way supported to remove efi_disk's even after >disconnect_controller() is called. > Do we want to remove disks? I don't really understand the context, but if EFI wants to forget about a disk, that should not affect the rest of U-Boot. > > My approach in this RFC: > > Due to functional differences in semantics, it would be difficult > to identify "udevice" structure as a handle in UEFI world. Instead, we will > have to somehow maintain a relationship between a udevice and a handle. > > 1-1. add a dedicated uclass, UCLASS_PARTITION, for partitions >Currently, the uclass for paritions is not a UCLASS_BLK. >It can be possible to define partitions as UCLASS_BLK >(with IF_TYPE_PARTION?), but >I'm afraid that it may introduce some chaos since udevice(UCLASS_BLK) >is tightly coupled with 'struct blk_desc' data which is still used >as a "structure to a whole disk" in a lot of interfaces. >(I hope that you understand what it means.) > >In DM tree, a UCLASS_PARTITON instance has a UCLASS_BLK par
Re: Re: Broken build with disabling OpenSSL crypto
Hi Alex!
Dne četrtek, 07. oktober 2021 ob 00:05:24 CEST je Alex G. napisal(a):
> Hi Jernej,
>
> On 10/6/21 4:27 PM, Jernej Škrabec wrote:
> > Hi everyone!
> >
> > Commit cb9faa6f98ae ("tools: Use a single target-independent config to
enable
> > OpenSSL") recently introduced option to disable usage of OpenSSL via
> > CONFIG_TOOLS_LIBCRYPTO. However, just a bit later, another commit
b4f3cc2c42d9
> > ("tools: kwbimage: Do not hide usage of secure header under
> > CONFIG_ARMADA_38X") made U-Boot tools hard dependent on OpenSSL. That
totally
> > defeats the purpose of first commit. I suggest that it gets reverted.
> >
> > I would like disable OpenSSL for my usage, since it gives me troubles when
> > cross-compiling U-Boot inside LibreELEC build system. It's not needed for
our
> > case anyway.
> >
> > Best regards,
> >
>
> Can you please give the following diff a try, and if it works for you, submit
as patch?
This works, I'll submit it as a patch. Should I keep you as original author
and add your SoB tag?
Best regards,
Jernej
>
> Alex
>
> diff --git a/tools/Makefile b/tools/Makefile
> index 4a86321f64..7f72ff9645 100644
> --- a/tools/Makefile
> +++ b/tools/Makefile
> @@ -96,7 +96,8 @@ AES_OBJS-$(CONFIG_TOOLS_LIBCRYPTO) := $(addprefix lib/aes/,
\
>
> # Cryptographic helpers that depend on openssl/libcrypto
> LIBCRYPTO_OBJS-$(CONFIG_TOOLS_LIBCRYPTO) := $(addprefix lib/, \
> - fdt-libcrypto.o)
> + fdt-libcrypto.o) \
> + kwbimage.o
>
> ROCKCHIP_OBS = lib/rc4.o rkcommon.o rkimage.o rksd.o rkspi.o
>
> @@ -117,7 +118,6 @@ dumpimage-mkimage-objs := aisimage.o \
> imximage.o \
> imx8image.o \
> imx8mimage.o \
> - kwbimage.o \
> lib/md5.o \
> lpc32xximage.o \
> mxsimage.o \
> @@ -169,8 +169,8 @@ HOST_EXTRACFLAGS += -
DCONFIG_FIT_SIGNATURE_MAX_SIZE=0x
> HOST_EXTRACFLAGS+= -DCONFIG_FIT_CIPHER
> endif
>
> -# MXSImage needs LibSSL
> -ifneq ($(CONFIG_MX23)$(CONFIG_MX28)$(CONFIG_ARMADA_38X)$
(CONFIG_TOOLS_LIBCRYPTO),)
> +# MXSImage needs LibSSL <- Nope! Read the frogging notice at the top
> +ifneq ($(CONFIG_TOOLS_LIBCRYPTO),)
> HOSTCFLAGS_kwbimage.o += \
> $(shell pkg-config --cflags libssl libcrypto 2> /dev/null || echo
"")
> HOSTLDLIBS_mkimage += \
>
Re: Re: Broken build with disabling OpenSSL crypto
Hi!
Dne četrtek, 07. oktober 2021 ob 21:41:00 CEST je Tom Rini napisal(a):
> On Wed, Oct 06, 2021 at 11:27:43PM +0200, Jernej Škrabec wrote:
>
> > Hi everyone!
> >
> > Commit cb9faa6f98ae ("tools: Use a single target-independent config to
enable
> > OpenSSL") recently introduced option to disable usage of OpenSSL via
> > CONFIG_TOOLS_LIBCRYPTO. However, just a bit later, another commit
b4f3cc2c42d9
> > ("tools: kwbimage: Do not hide usage of secure header under
> > CONFIG_ARMADA_38X") made U-Boot tools hard dependent on OpenSSL. That
totally
> > defeats the purpose of first commit. I suggest that it gets reverted.
> >
> > I would like disable OpenSSL for my usage, since it gives me troubles when
> > cross-compiling U-Boot inside LibreELEC build system. It's not needed for
our
> > case anyway.
>
> How hard is it to specify openssl as a dependency for U-Boot, in the
> LibreELEC build system? I assume openssl is being used in other parts
> of the build anyhow. Thanks!
Sure, OpenSSL package is present and we fixed issue with pkg-config in the
meantime (it picked target paths instead for the host). However, that doesn't
change anything. CONFIG_TOOLS_LIBCRYPTO is still borked and should be either
fixed or dropped. I prefer first option because OpenSSL dependency can be
removed and that allows more concurrency (multiple packages are built at the
same time).
Best regards,
Jernej
Re: [PATCH 1/7] arm: mach-imx: Update MAC fuse for i.MX8MP
On Mon, 2021-08-16 at 18:44 +0800, Ye Li wrote:
> i.MX8MP has two ENET controllers, have to update the function to
> enable loading two MAC addresses.
>
> Signed-off-by: Ye Li
Whole series.
Tested on iMX8MP_EVK and Verdin iMX8M Plus on Verdin development board.
Tested-by: Marcel Ziswiler
I will post the Verdin iMX8M Plus support shortly.
> ---
> arch/arm/mach-imx/mac.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm/mach-imx/mac.c b/arch/arm/mach-imx/mac.c
> index 3b1496b..9bb63d2 100644
> --- a/arch/arm/mach-imx/mac.c
> +++ b/arch/arm/mach-imx/mac.c
> @@ -31,7 +31,7 @@ void imx_get_mac_from_fuse(int dev_id, unsigned char *mac)
>
> offset = is_mx6() ? MAC_FUSE_MX6_OFFSET : MAC_FUSE_MX7_OFFSET;
> fuse = (struct imx_mac_fuse *)(ulong)(OCOTP_BASE_ADDR + offset);
> - has_second_mac = is_mx7() || is_mx6sx() || is_mx6ul() || is_mx6ull();
> + has_second_mac = is_mx7() || is_mx6sx() || is_mx6ul() || is_mx6ull()
> || is_imx8mp();
>
> if (has_second_mac && dev_id == 1) {
> u32 value = readl(&fuse->mac_addr2);
