All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrei Warkentin <awarkentin@vmware.com>
To: Namjae Jeon <linkinjeon@gmail.com>
Cc: linux-kernel@vger.kernel.org, cjb@laptop.org, linux-mmc@vger.kernel.org
Subject: Re: [PATCH] mmc : general purpose partition support.
Date: Thu, 22 Sep 2011 20:25:25 -0700 (PDT)	[thread overview]
Message-ID: <728441958.616721.1316748325054.JavaMail.root@zimbra-prod-mbox-2.vmware.com> (raw)
In-Reply-To: <1316705680-1940-1-git-send-email-linkinjeon@gmail.com>

Hi Namjae,

I scanned over your changes to enable GP partitions, and while I 
did not yet test your changes, they do appear to be ok.

Please CC me on the next version of your patch so I can give
it a spin and Ack on it.

I do suggest at least thinking about ways to improve on this. Back
when I added boot partition support, I already didn't like the fact
that the block driver had to be aware of the eMMC partitions, with all
the code duplication and such. By adding four more partitions,
you've compounded the problem and now you have six pieces of 
code that really look the same.

The core/mmc.c right now scans the size values and stores them
in mmc_card, and the block driver is smart enough to know what
to do with them. Why not make the block code generic at the expense
of keeping an array of structures in mmc_card? The block driver
would then just iterate over these and create the additional devices. The partition
switch routine would then operate on these structures, and could then be pulled out
of the block driver, making it simpler. 

Each physical partition could be described by something like - 
struct mmc_part {
    unsigned int size;
    unsigned int cookie;        /* for mmc, idx used in mmc_switch, part_type from current mmc_blk_data */
    char name[DISK_NAME_LEN];
    bool force_ro;              /* to make boot parts RO by default */
};

Just an idea. Clearly, if MMC grows by another 4-5 possible partitions, it's not
realistic to be copy-pasting the same block of code, nor would it make sense
if/when these types of devices support an arbitrary amount of physical partitions.

Thanks,
A

  parent reply	other threads:[~2011-09-23  3:25 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-09-22 15:34 [PATCH] mmc : general purpose partition support Namjae Jeon
2011-09-22 22:08 ` J Freyensee
2011-09-22 23:15   ` NamJae Jeon
2011-09-22 23:15     ` NamJae Jeon
2011-09-22 23:25     ` J Freyensee
2011-09-22 23:58       ` NamJae Jeon
2011-09-22 23:58         ` NamJae Jeon
2011-09-23  0:19         ` J Freyensee
2011-09-23  0:29           ` NamJae Jeon
2011-09-23  0:29             ` NamJae Jeon
     [not found]             ` <4E7CB5FC.3090301@linux.intel.com>
2011-09-24  5:21               ` NamJae Jeon
2011-09-24  5:21                 ` NamJae Jeon
2011-09-23  3:25 ` Andrei Warkentin [this message]
2011-09-23  3:56   ` NamJae Jeon
2011-10-04 16:16 Namjae Jeon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=728441958.616721.1316748325054.JavaMail.root@zimbra-prod-mbox-2.vmware.com \
    --to=awarkentin@vmware.com \
    --cc=cjb@laptop.org \
    --cc=linkinjeon@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mmc@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.