On Tue, Oct 18, 2016 at 08:23:22AM +0200, Sascha Hauer wrote:
> On Mon, Oct 17, 2016 at 03:29:23PM +0200, Michael Grzeschik wrote:
> > With this patch it is possible to write an mbr partition table to the
> > mci block device. By setting the device property "dos_partitions" of the
> > mmc device node, it is possible to write back the new partition layout
> > in the common cmdlinepart notation. The property can also be read back.
> >
> > Signed-off-by: Michael Grzeschik <m.grzeschik@xxxxxxxxxxxxxx>
> > ---
> > drivers/mci/mci-core.c | 122 +++++++++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 122 insertions(+)
> >
> > diff --git a/drivers/mci/mci-core.c b/drivers/mci/mci-core.c
> > index 4e176f7..c0013a1 100644
> > --- a/drivers/mci/mci-core.c
> > +++ b/drivers/mci/mci-core.c
> > @@ -33,9 +33,11 @@
> > #include <asm-generic/div64.h>
> > #include <asm/byteorder.h>
> > #include <block.h>
> > +#include <fcntl.h>
> > #include <disks.h>
> > #include <of.h>
> > #include <linux/err.h>
> > +#include <cmdlinepart.h>
> >
> > #define MAX_BUFFER_NUMBER 0xffffffff
> >
> > @@ -1527,6 +1529,122 @@ static void mci_info(struct device_d *dev)
> > extract_mtd_year(mci));
> > }
> >
> > +static char *print_size(uint64_t s)
> > +{
> > + if (!(s & ((1 << 20) - 1)))
> > + return basprintf("%lldM", s >> 20);
> > + if (!(s & ((1 << 10) - 1)))
> > + return basprintf("%lldk", s >> 10);
> > + return basprintf("0x%lld", s);
>
> s/lld/llx/
Why that? This will break the typical layout compared
to all other users of the kernelcmdline syntax.
> > +}
> > +
> > +static int print_part(char *buf, int bufsize, struct cdev *cdev, int is_last)
> > +{
> > + char *size = print_size(cdev->size);
> > + int ret;
> > +
> > + if (!size) {
> > + ret = -ENOMEM;
> > + goto out;
> > + }
> > +
> > + ret = snprintf(buf, bufsize, "%s(%s)%s", size,
> > + cdev->partname,
> > + is_last ? "" : ",");
> > +out:
> > + free(size);
> > +
> > + return ret;
> > +}
> > +
> > +static int print_parts(char *buf, int bufsize, struct mci *mci)
> > +{
> > + struct cdev *cdev, *ct;
> > + int ret = 0;
> > +
> > + list_for_each_entry_safe(cdev, ct, &mci->dev.cdevs, devices_list) {
>
> safe_?
Sure. We don't change the list.
> > + if ((cdev->flags & DEVFS_IS_PARTITION) &&
> > + (cdev->flags & DEVFS_PARTITION_IN_PT)) {
> > + int now;
> > + int is_last = 0;
> > + struct list_head *nh = (cdev)->devices_list.next;
> > + struct cdev *next = container_of(nh, typeof(*(cdev)), devices_list);
> > +
> > + if (list_is_last(&cdev->devices_list, &mci->dev.cdevs) ||
> > + !(next->flags & DEVFS_PARTITION_IN_PT))
> > + is_last = 1;
>
> Is this test safe? What if the next partition does not have the
> DEVFS_PARTITION_IN_PT flag set, but the one after that has? Maybe you
> have to count the number of partitions in a first pass.
Yes. That's a good point. Will fix that.
> > +
> > + now = print_part(buf, bufsize, cdev, is_last);
> > + if (now < 0)
> > + return now;
> > +
> > + if (buf && bufsize) {
> > + buf += now;
> > + bufsize -= now;
> > + }
> > + ret += now;
> > + }
> > + }
> > +
> > + return ret;
> > +}
> > +
> > +static const char *mci_partition_get(struct device_d *dev, struct param_d *p)
> > +{
> > + struct mci *mci = container_of(dev, struct mci, dev);
> > + int len = 0;
> > +
> > + free(p->value);
> > +
> > + len = print_parts(NULL, 0, mci);
> > + p->value = xzalloc(len + 1);
> > + print_parts(p->value, len + 1, mci);
> > +
> > + return p->value;
> > +}
> > +
> > +#ifdef CONFIG_BLOCK_WRITE
> > +static int mci_partition_set(struct device_d *dev, struct param_d *p, const char *val)
> > +{
> > + struct mci *mci = container_of(dev, struct mci, dev);
> > + struct cdev *cdev, *ct;
> > + int ret;
> > +
> > + if (!val)
> > + return -EINVAL;
> > +
> > + /* remove all partition cdevs with DEVFS_IS_PARTITION set */
> > + list_for_each_entry_safe(cdev, ct, &mci->dev.cdevs, devices_list) {
> > + if ((cdev->flags & DEVFS_IS_PARTITION) &&
> > + (cdev->flags & DEVFS_PARTITION_IN_PT))
> > + ret = devfs_del_partition(cdev->name);
> > + if (ret)
> > + return ret;
> > + }
> > +
> > + /* read back the prepared partition layot from dos_partitions param */
>
> s/layot/layout/
>
Jupp.
> > + ret = cmdlinepart_do_parse(mci->cdevname, val, mci->capacity,
> > + CMDLINEPART_ADD_DEVNAME | CMDLINEPART_ADD_TO_PT);
> > + if (ret)
> > + return ret;
> > +
> > + /* write the MBR partition layout based on cdevs with DEVFS_IS_PARTITION set */
> > + for (int i = 0; i < mci->nr_parts; i++) {
> > + struct mci_part *part = &mci->part[i];
> > + if (part->area_type == MMC_BLK_DATA_AREA_MAIN) {
> > + ret = write_dos_partition_table(&part->blk,
> > + &mci->dev.cdevs);
> > + if (ret != 0) {
> > + dev_warn(&mci->dev, "Could not write partition table\n");
> > + return ret;
> > + }
> > + }
> > + }
> > +
> > + return ret;
> > +}
> > +#endif
> > +
> > /**
> > * Check if the MCI card is already probed
> > * @param mci MCI device instance
> > @@ -1786,6 +1904,10 @@ int mci_register(struct mci_host *host)
> > mci->param_probe = dev_add_param_bool(&mci->dev, "probe",
> > mci_set_probe, NULL, &mci->probe, mci);
> >
> > +#ifdef CONFIG_BLOCK_WRITE
> > + dev_add_param(&mci->dev, "dos_partitions", mci_partition_set, mci_partition_get, 0);
> > +#endif
>
> Use IS_ENABLED()
>
> Other than that this code should be attached to parse_partition_table()
> rather than making this mci specific.
> We probably can safely write a dos partition table to an unpartitioned
> device, but should refuse to create/manipulate a dos partition table
> when a EFI partition table exists.
I will have that fixed in v2, until I figured out how to integrate that properly.
Thanks,
Michael
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
_______________________________________________
barebox mailing list
barebox@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/barebox
