Re: [PATCH 4/4] media: imx-media-capture: add frame sizes/interval enumeration
On Mon, Mar 20, 2017 at 09:05:25AM +, Russell King - ARM Linux wrote:
> On Mon, Mar 20, 2017 at 09:55:12AM +0100, Philippe De Muyter wrote:
> > Hi Russel,
> >
> > On Sun, Mar 19, 2017 at 10:49:08AM +, Russell King wrote:
> > > Add support for enumerating frame sizes and frame intervals from the
> > > first subdev via the V4L2 interfaces.
> > >
> > > Signed-off-by: Russell King <rmk+ker...@armlinux.org.uk>
> > > ---
> > > drivers/staging/media/imx/imx-media-capture.c | 62
> > > +++
> > > 1 file changed, 62 insertions(+)
> > >
> > ...
> > > +static int capture_enum_frameintervals(struct file *file, void *fh,
> > > +struct v4l2_frmivalenum *fival)
> > > +{
> > > + struct capture_priv *priv = video_drvdata(file);
> > > + const struct imx_media_pixfmt *cc;
> > > + struct v4l2_subdev_frame_interval_enum fie = {
> > > + .index = fival->index,
> > > + .pad = priv->src_sd_pad,
> > > + .width = fival->width,
> > > + .height = fival->height,
> > > + .which = V4L2_SUBDEV_FORMAT_ACTIVE,
> > > + };
> > > + int ret;
> > > +
> > > + cc = imx_media_find_format(fival->pixel_format, CS_SEL_ANY, true);
> > > + if (!cc)
> > > + return -EINVAL;
> > > +
> > > + fie.code = cc->codes[0];
> > > +
> > > + ret = v4l2_subdev_call(priv->src_sd, pad, enum_frame_interval, NULL,
> > > );
> > > + if (ret)
> > > + return ret;
> > > +
> > > + fival->type = V4L2_FRMIVAL_TYPE_DISCRETE;
> > > + fival->discrete = fie.interval;
> >
> > For some parallel sensors (mine is a E2V ev76c560) "any" frame interval is
> > possible,
> > and hence type should be V4L2_FRMIVAL_TYPE_CONTINUOUS.
>
> For my sensor, any frame interval is also possible, but that isn't the
> point here.
>
> /dev/video* only talks to the CSI source pad, not it's sink pad. The
> sink pad gets configured with the sensor frame rate via the media
> controller API. /dev/video* itself has no control over the sensor
> frame rate.
>
> The media controller stuff completely changes the way the established
> /dev/video* functionality works - the ability to select arbitary frame
> sizes and frame rates supported by the ultimate sensor is gone. All
> that needs to be setup through the media controller pipeline, one
> subdev at a time.
>
So existing gstreamer applications using /dev/video* to control framerate,
and even gain and exposure won't work anymore :( ?
I had hoped to keep compatibility, with added robustness and functionality.
I seems like I'll stay with my NXP/Freescale old and imperfect kernel.
Best regards
Philippe
--
Philippe De Muyter +32 2 6101532 Macq SA rue de l'Aeronef 2 B-1140 Bruxelles
Re: [PATCH 4/4] media: imx-media-capture: add frame sizes/interval enumeration
On Mon, Mar 20, 2017 at 09:05:25AM +, Russell King - ARM Linux wrote:
> On Mon, Mar 20, 2017 at 09:55:12AM +0100, Philippe De Muyter wrote:
> > Hi Russel,
> >
> > On Sun, Mar 19, 2017 at 10:49:08AM +, Russell King wrote:
> > > Add support for enumerating frame sizes and frame intervals from the
> > > first subdev via the V4L2 interfaces.
> > >
> > > Signed-off-by: Russell King
> > > ---
> > > drivers/staging/media/imx/imx-media-capture.c | 62
> > > +++
> > > 1 file changed, 62 insertions(+)
> > >
> > ...
> > > +static int capture_enum_frameintervals(struct file *file, void *fh,
> > > +struct v4l2_frmivalenum *fival)
> > > +{
> > > + struct capture_priv *priv = video_drvdata(file);
> > > + const struct imx_media_pixfmt *cc;
> > > + struct v4l2_subdev_frame_interval_enum fie = {
> > > + .index = fival->index,
> > > + .pad = priv->src_sd_pad,
> > > + .width = fival->width,
> > > + .height = fival->height,
> > > + .which = V4L2_SUBDEV_FORMAT_ACTIVE,
> > > + };
> > > + int ret;
> > > +
> > > + cc = imx_media_find_format(fival->pixel_format, CS_SEL_ANY, true);
> > > + if (!cc)
> > > + return -EINVAL;
> > > +
> > > + fie.code = cc->codes[0];
> > > +
> > > + ret = v4l2_subdev_call(priv->src_sd, pad, enum_frame_interval, NULL,
> > > );
> > > + if (ret)
> > > + return ret;
> > > +
> > > + fival->type = V4L2_FRMIVAL_TYPE_DISCRETE;
> > > + fival->discrete = fie.interval;
> >
> > For some parallel sensors (mine is a E2V ev76c560) "any" frame interval is
> > possible,
> > and hence type should be V4L2_FRMIVAL_TYPE_CONTINUOUS.
>
> For my sensor, any frame interval is also possible, but that isn't the
> point here.
>
> /dev/video* only talks to the CSI source pad, not it's sink pad. The
> sink pad gets configured with the sensor frame rate via the media
> controller API. /dev/video* itself has no control over the sensor
> frame rate.
>
> The media controller stuff completely changes the way the established
> /dev/video* functionality works - the ability to select arbitary frame
> sizes and frame rates supported by the ultimate sensor is gone. All
> that needs to be setup through the media controller pipeline, one
> subdev at a time.
>
So existing gstreamer applications using /dev/video* to control framerate,
and even gain and exposure won't work anymore :( ?
I had hoped to keep compatibility, with added robustness and functionality.
I seems like I'll stay with my NXP/Freescale old and imperfect kernel.
Best regards
Philippe
--
Philippe De Muyter +32 2 6101532 Macq SA rue de l'Aeronef 2 B-1140 Bruxelles
Re: [PATCH 4/4] media: imx-media-capture: add frame sizes/interval enumeration
Hi Russel,
On Sun, Mar 19, 2017 at 10:49:08AM +, Russell King wrote:
> Add support for enumerating frame sizes and frame intervals from the
> first subdev via the V4L2 interfaces.
>
> Signed-off-by: Russell King <rmk+ker...@armlinux.org.uk>
> ---
> drivers/staging/media/imx/imx-media-capture.c | 62
> +++
> 1 file changed, 62 insertions(+)
>
...
> +static int capture_enum_frameintervals(struct file *file, void *fh,
> +struct v4l2_frmivalenum *fival)
> +{
> + struct capture_priv *priv = video_drvdata(file);
> + const struct imx_media_pixfmt *cc;
> + struct v4l2_subdev_frame_interval_enum fie = {
> + .index = fival->index,
> + .pad = priv->src_sd_pad,
> + .width = fival->width,
> + .height = fival->height,
> + .which = V4L2_SUBDEV_FORMAT_ACTIVE,
> + };
> + int ret;
> +
> + cc = imx_media_find_format(fival->pixel_format, CS_SEL_ANY, true);
> + if (!cc)
> + return -EINVAL;
> +
> + fie.code = cc->codes[0];
> +
> + ret = v4l2_subdev_call(priv->src_sd, pad, enum_frame_interval, NULL,
> );
> + if (ret)
> + return ret;
> +
> + fival->type = V4L2_FRMIVAL_TYPE_DISCRETE;
> + fival->discrete = fie.interval;
For some parallel sensors (mine is a E2V ev76c560) "any" frame interval is
possible,
and hence type should be V4L2_FRMIVAL_TYPE_CONTINUOUS.
see also https://www.spinics.net/lists/linux-media/msg98622.html,
https://patchwork.kernel.org/patch/9171201/ and
https://patchwork.kernel.org/patch/9171199/
Philippe
--
Philippe De Muyter +32 2 6101532 Macq SA rue de l'Aeronef 2 B-1140 Bruxelles
Re: [PATCH 4/4] media: imx-media-capture: add frame sizes/interval enumeration
Hi Russel,
On Sun, Mar 19, 2017 at 10:49:08AM +, Russell King wrote:
> Add support for enumerating frame sizes and frame intervals from the
> first subdev via the V4L2 interfaces.
>
> Signed-off-by: Russell King
> ---
> drivers/staging/media/imx/imx-media-capture.c | 62
> +++
> 1 file changed, 62 insertions(+)
>
...
> +static int capture_enum_frameintervals(struct file *file, void *fh,
> +struct v4l2_frmivalenum *fival)
> +{
> + struct capture_priv *priv = video_drvdata(file);
> + const struct imx_media_pixfmt *cc;
> + struct v4l2_subdev_frame_interval_enum fie = {
> + .index = fival->index,
> + .pad = priv->src_sd_pad,
> + .width = fival->width,
> + .height = fival->height,
> + .which = V4L2_SUBDEV_FORMAT_ACTIVE,
> + };
> + int ret;
> +
> + cc = imx_media_find_format(fival->pixel_format, CS_SEL_ANY, true);
> + if (!cc)
> + return -EINVAL;
> +
> + fie.code = cc->codes[0];
> +
> + ret = v4l2_subdev_call(priv->src_sd, pad, enum_frame_interval, NULL,
> );
> + if (ret)
> + return ret;
> +
> + fival->type = V4L2_FRMIVAL_TYPE_DISCRETE;
> + fival->discrete = fie.interval;
For some parallel sensors (mine is a E2V ev76c560) "any" frame interval is
possible,
and hence type should be V4L2_FRMIVAL_TYPE_CONTINUOUS.
see also https://www.spinics.net/lists/linux-media/msg98622.html,
https://patchwork.kernel.org/patch/9171201/ and
https://patchwork.kernel.org/patch/9171199/
Philippe
--
Philippe De Muyter +32 2 6101532 Macq SA rue de l'Aeronef 2 B-1140 Bruxelles
Re: media / v4l2-mc: wishlist for complex cameras (was Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit controls from a pipeline)
On Thu, Mar 16, 2017 at 11:01:56AM +0100, Philipp Zabel wrote: > On Thu, 2017-03-16 at 10:47 +0100, Philippe De Muyter wrote: > > On Thu, Mar 16, 2017 at 10:26:00AM +0100, Philipp Zabel wrote: > > > On Wed, 2017-03-15 at 14:55 -0400, Nicolas Dufresne wrote: > > > > Le mercredi 15 mars 2017 à 11:50 +0100, Philippe De Muyter a écrit : > > > > > > I would say: camorama, xawtv3, zbar, google talk, skype. If it runs > > > > > > with those, it will likely run with any other application. > > > > > > > > > > > > > > > > I would like to add the 'v4l2src' plugin of gstreamer, and on the > > > > > imx6 its > > > > > > > > While it would be nice if somehow you would get v4l2src to work (in > > > > some legacy/emulation mode through libv4l2), > > > > > > v4l2src works just fine, provided the pipeline is configured manually in > > > advance via media-ctl. > > > > Including choosing the framerate ? Sorry, I have no time these days > > to test it myself. > > No, the framerate is set with media-ctl on the CSI output pad. To really > choose the framerate, the element would indeed need a deeper > understanding of the pipeline, as the resulting framerate depends on at > least the source v4l2_subdevice (sensor) framerate and the CSI frame > skipping. Count me in than as a supporter of Steve's "v4l2-mc: add a function to inherit controls from a pipeline" patch > > of the image buffers for further processing by other (not necessarily next > > in gstreamer pipeline, or for all frames) hardware-accelerated plugins likes > > the h.264 video encoder. As I am stuck with fsl/nxp kernel and driver on > > that > > matter, I don't know how the interfaces have evolved in current linux > > kernels. > > The physical address of the image buffers is hidden from userspace by > dma-buf objects, but those can be passed around to the next driver > without copying the image data. OK thanks Philippe
Re: media / v4l2-mc: wishlist for complex cameras (was Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit controls from a pipeline)
On Thu, Mar 16, 2017 at 11:01:56AM +0100, Philipp Zabel wrote: > On Thu, 2017-03-16 at 10:47 +0100, Philippe De Muyter wrote: > > On Thu, Mar 16, 2017 at 10:26:00AM +0100, Philipp Zabel wrote: > > > On Wed, 2017-03-15 at 14:55 -0400, Nicolas Dufresne wrote: > > > > Le mercredi 15 mars 2017 à 11:50 +0100, Philippe De Muyter a écrit : > > > > > > I would say: camorama, xawtv3, zbar, google talk, skype. If it runs > > > > > > with those, it will likely run with any other application. > > > > > > > > > > > > > > > > I would like to add the 'v4l2src' plugin of gstreamer, and on the > > > > > imx6 its > > > > > > > > While it would be nice if somehow you would get v4l2src to work (in > > > > some legacy/emulation mode through libv4l2), > > > > > > v4l2src works just fine, provided the pipeline is configured manually in > > > advance via media-ctl. > > > > Including choosing the framerate ? Sorry, I have no time these days > > to test it myself. > > No, the framerate is set with media-ctl on the CSI output pad. To really > choose the framerate, the element would indeed need a deeper > understanding of the pipeline, as the resulting framerate depends on at > least the source v4l2_subdevice (sensor) framerate and the CSI frame > skipping. Count me in than as a supporter of Steve's "v4l2-mc: add a function to inherit controls from a pipeline" patch > > of the image buffers for further processing by other (not necessarily next > > in gstreamer pipeline, or for all frames) hardware-accelerated plugins likes > > the h.264 video encoder. As I am stuck with fsl/nxp kernel and driver on > > that > > matter, I don't know how the interfaces have evolved in current linux > > kernels. > > The physical address of the image buffers is hidden from userspace by > dma-buf objects, but those can be passed around to the next driver > without copying the image data. OK thanks Philippe
Re: media / v4l2-mc: wishlist for complex cameras (was Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit controls from a pipeline)
On Thu, Mar 16, 2017 at 10:26:00AM +0100, Philipp Zabel wrote: > On Wed, 2017-03-15 at 14:55 -0400, Nicolas Dufresne wrote: > > Le mercredi 15 mars 2017 à 11:50 +0100, Philippe De Muyter a écrit : > > > > I would say: camorama, xawtv3, zbar, google talk, skype. If it runs > > > > with those, it will likely run with any other application. > > > > > > > > > > I would like to add the 'v4l2src' plugin of gstreamer, and on the > > > imx6 its > > > > While it would be nice if somehow you would get v4l2src to work (in > > some legacy/emulation mode through libv4l2), > > v4l2src works just fine, provided the pipeline is configured manually in > advance via media-ctl. Including choosing the framerate ? Sorry, I have no time these days to test it myself. And I cited imxv4l2videosrc for its ability to provide the physical address of the image buffers for further processing by other (not necessarily next in gstreamer pipeline, or for all frames) hardware-accelerated plugins likes the h.264 video encoder. As I am stuck with fsl/nxp kernel and driver on that matter, I don't know how the interfaces have evolved in current linux kernels. BR Philippe -- Philippe De Muyter +32 2 6101532 Macq SA rue de l'Aeronef 2 B-1140 Bruxelles
Re: media / v4l2-mc: wishlist for complex cameras (was Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit controls from a pipeline)
On Thu, Mar 16, 2017 at 10:26:00AM +0100, Philipp Zabel wrote: > On Wed, 2017-03-15 at 14:55 -0400, Nicolas Dufresne wrote: > > Le mercredi 15 mars 2017 à 11:50 +0100, Philippe De Muyter a écrit : > > > > I would say: camorama, xawtv3, zbar, google talk, skype. If it runs > > > > with those, it will likely run with any other application. > > > > > > > > > > I would like to add the 'v4l2src' plugin of gstreamer, and on the > > > imx6 its > > > > While it would be nice if somehow you would get v4l2src to work (in > > some legacy/emulation mode through libv4l2), > > v4l2src works just fine, provided the pipeline is configured manually in > advance via media-ctl. Including choosing the framerate ? Sorry, I have no time these days to test it myself. And I cited imxv4l2videosrc for its ability to provide the physical address of the image buffers for further processing by other (not necessarily next in gstreamer pipeline, or for all frames) hardware-accelerated plugins likes the h.264 video encoder. As I am stuck with fsl/nxp kernel and driver on that matter, I don't know how the interfaces have evolved in current linux kernels. BR Philippe -- Philippe De Muyter +32 2 6101532 Macq SA rue de l'Aeronef 2 B-1140 Bruxelles
Re: media / v4l2-mc: wishlist for complex cameras (was Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit controls from a pipeline)
On Tue, Mar 14, 2017 at 09:54:31PM -0300, Mauro Carvalho Chehab wrote: > Em Tue, 14 Mar 2017 23:32:54 +0100 > Pavel Machek <pa...@ucw.cz> escreveu: > > > > > Well, I believe first question is: what applications would we want to > > run on complex devices? Will sending control from video to subdevs > > actually help? > > I would say: camorama, xawtv3, zbar, google talk, skype. If it runs > with those, it will likely run with any other application. > I would like to add the 'v4l2src' plugin of gstreamer, and on the imx6 its imx-specific counterpart 'imxv4l2videosrc' from the gstreamer-imx package at https://github.com/Freescale/gstreamer-imx, and 'v4l2-ctl'. Philippe -- Philippe De Muyter +32 2 6101532 Macq SA rue de l'Aeronef 2 B-1140 Bruxelles
Re: media / v4l2-mc: wishlist for complex cameras (was Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit controls from a pipeline)
On Tue, Mar 14, 2017 at 09:54:31PM -0300, Mauro Carvalho Chehab wrote: > Em Tue, 14 Mar 2017 23:32:54 +0100 > Pavel Machek escreveu: > > > > > Well, I believe first question is: what applications would we want to > > run on complex devices? Will sending control from video to subdevs > > actually help? > > I would say: camorama, xawtv3, zbar, google talk, skype. If it runs > with those, it will likely run with any other application. > I would like to add the 'v4l2src' plugin of gstreamer, and on the imx6 its imx-specific counterpart 'imxv4l2videosrc' from the gstreamer-imx package at https://github.com/Freescale/gstreamer-imx, and 'v4l2-ctl'. Philippe -- Philippe De Muyter +32 2 6101532 Macq SA rue de l'Aeronef 2 B-1140 Bruxelles
Re: [PATCH] m68k: Fix ndelay() macro
On Fri, Oct 28, 2016 at 05:12:28PM +0200, Boris Brezillon wrote: > The current ndelay() macro definition has an extra semi-colon at the > end of the line thus leading to a compilation error when ndelay is used > in a conditional block with curly braces like this one: without curly braces > > if (cond) > ndelay(t); > else > ... > Philippe -- Philippe De Muyter +32 2 6101532 Macq SA rue de l'Aeronef 2 B-1140 Bruxelles
Re: [PATCH] m68k: Fix ndelay() macro
On Fri, Oct 28, 2016 at 05:12:28PM +0200, Boris Brezillon wrote: > The current ndelay() macro definition has an extra semi-colon at the > end of the line thus leading to a compilation error when ndelay is used > in a conditional block with curly braces like this one: without curly braces > > if (cond) > ndelay(t); > else > ... > Philippe -- Philippe De Muyter +32 2 6101532 Macq SA rue de l'Aeronef 2 B-1140 Bruxelles
Re: [PATCH 08/10] m68k: Add
On Wed, May 25, 2016 at 03:34:55AM -0400, George Spelvin wrote: > This provides a multiply by constant GOLDEN_RATIO_32 = 0x61C88647 > for the original mc68000, which lacks a 32x32-bit multiply instruction. > > Addition chains found by Yevgen Voronenko's Hcub algorithm at > http://spiral.ece.cmu.edu/mcm/gen.html Shouldn't you put that reference in the comments of your archhash.h file ? Philippe
Re: [PATCH 08/10] m68k: Add
On Wed, May 25, 2016 at 03:34:55AM -0400, George Spelvin wrote: > This provides a multiply by constant GOLDEN_RATIO_32 = 0x61C88647 > for the original mc68000, which lacks a 32x32-bit multiply instruction. > > Addition chains found by Yevgen Voronenko's Hcub algorithm at > http://spiral.ece.cmu.edu/mcm/gen.html Shouldn't you put that reference in the comments of your archhash.h file ? Philippe
Re: [PATCH 08/10] m68k: Add
On Wed, May 25, 2016 at 05:14:35AM -0400, George Spelvin wrote:
>
> I did it the way I did above because it makes the gcc -S output very
> legible. Just like I put a space before the perands on m68k but a tab
> on h8300: that's what GCC does on those platforms.
>
> I started with the "\n\t" suffixes on each line like so much other
> kernel code, but then figured out the format above which is legible
> both in C source and compiler output.
OK thanks.
>
> >>asm("move.l %2,%0;" /* 0x0001 */
> >>"lsl.l #2,%0;" /* 0x0004 */
> >>"move.l %0,%1;"
> >>"lsl.l #7,%0;" /* 0x0200 */
> >>"add.l %2,%0;" /* 0x0201 */
> >>"add.l %0,%1;" /* 0x0205 */
> >>"add.l %0,%0;" /* 0x0402 */
> >>"add.l %0,%1;" /* 0x0607 */
> >>"lsl.l #5,%0" /* 0x8040 */
> >>/* 0x8647 */
>
> > Also, it took me some time to understand the hexadecimal constants
> > in the comments (and the last one predicts a future event :)).
>
>
> Can you recmmend a better way to comment this? My nose is so deep
> in the code it's hard for me to judge.
I second Andreas' suggestion.
Philippe
--
Philippe De Muyter +32 2 6101532 Macq SA rue de l'Aeronef 2 B-1140 Bruxelles
Re: [PATCH 08/10] m68k: Add
On Wed, May 25, 2016 at 05:14:35AM -0400, George Spelvin wrote:
>
> I did it the way I did above because it makes the gcc -S output very
> legible. Just like I put a space before the perands on m68k but a tab
> on h8300: that's what GCC does on those platforms.
>
> I started with the "\n\t" suffixes on each line like so much other
> kernel code, but then figured out the format above which is legible
> both in C source and compiler output.
OK thanks.
>
> >>asm("move.l %2,%0;" /* 0x0001 */
> >>"lsl.l #2,%0;" /* 0x0004 */
> >>"move.l %0,%1;"
> >>"lsl.l #7,%0;" /* 0x0200 */
> >>"add.l %2,%0;" /* 0x0201 */
> >>"add.l %0,%1;" /* 0x0205 */
> >>"add.l %0,%0;" /* 0x0402 */
> >>"add.l %0,%1;" /* 0x0607 */
> >>"lsl.l #5,%0" /* 0x8040 */
> >>/* 0x8647 */
>
> > Also, it took me some time to understand the hexadecimal constants
> > in the comments (and the last one predicts a future event :)).
>
>
> Can you recmmend a better way to comment this? My nose is so deep
> in the code it's hard for me to judge.
I second Andreas' suggestion.
Philippe
--
Philippe De Muyter +32 2 6101532 Macq SA rue de l'Aeronef 2 B-1140 Bruxelles
Re: [PATCH 08/10] m68k: Add
On Wed, May 25, 2016 at 03:34:55AM -0400, George Spelvin wrote:
> +static inline u32 __attribute_const__ __hash_32(u32 x)
> +{
> + u32 a, b;
> +
> + asm( "move.l %2,%0" /* 0x0001 */
> + "\n lsl.l #2,%0"/* 0x0004 */
> + "\n move.l %0,%1"
> + "\n lsl.l #7,%0"/* 0x0200 */
> + "\n add.l %2,%0"/* 0x0201 */
> + "\n add.l %0,%1"/* 0x0205 */
> + "\n add.l %0,%0"/* 0x0402 */
> + "\n add.l %0,%1"/* 0x0607 */
> + "\n lsl.l #5,%0"/* 0x8040 */
> + /* 0x8647 */
There is no standard way to write asm in the kernel, but I prefer
a simple semicolon after each insn
asm("move.l %2,%0;" /* 0x0001 */
"lsl.l #2,%0;" /* 0x0004 */
"move.l %0,%1;"
"lsl.l #7,%0;" /* 0x0200 */
"add.l %2,%0;" /* 0x0201 */
"add.l %0,%1;" /* 0x0205 */
"add.l %0,%0;" /* 0x0402 */
"add.l %0,%1;" /* 0x0607 */
"lsl.l #5,%0" /* 0x8040 */
/* 0x8647 */
Also, it took me some time to understand the hexadecimal constants
in the comments (and the last one predicts a future event :)).
> + : "=" (a), "=" (b)
> + : "g" (x));
> +
> + return ((u16)(x*0x61c8) << 16) + a + b;
> +}
Just my two cents
Philippe
Re: [PATCH 08/10] m68k: Add
On Wed, May 25, 2016 at 03:34:55AM -0400, George Spelvin wrote:
> +static inline u32 __attribute_const__ __hash_32(u32 x)
> +{
> + u32 a, b;
> +
> + asm( "move.l %2,%0" /* 0x0001 */
> + "\n lsl.l #2,%0"/* 0x0004 */
> + "\n move.l %0,%1"
> + "\n lsl.l #7,%0"/* 0x0200 */
> + "\n add.l %2,%0"/* 0x0201 */
> + "\n add.l %0,%1"/* 0x0205 */
> + "\n add.l %0,%0"/* 0x0402 */
> + "\n add.l %0,%1"/* 0x0607 */
> + "\n lsl.l #5,%0"/* 0x8040 */
> + /* 0x8647 */
There is no standard way to write asm in the kernel, but I prefer
a simple semicolon after each insn
asm("move.l %2,%0;" /* 0x0001 */
"lsl.l #2,%0;" /* 0x0004 */
"move.l %0,%1;"
"lsl.l #7,%0;" /* 0x0200 */
"add.l %2,%0;" /* 0x0201 */
"add.l %0,%1;" /* 0x0205 */
"add.l %0,%0;" /* 0x0402 */
"add.l %0,%1;" /* 0x0607 */
"lsl.l #5,%0" /* 0x8040 */
/* 0x8647 */
Also, it took me some time to understand the hexadecimal constants
in the comments (and the last one predicts a future event :)).
> + : "=" (a), "=" (b)
> + : "g" (x));
> +
> + return ((u16)(x*0x61c8) << 16) + a + b;
> +}
Just my two cents
Philippe
[PATCH] [media] gpu: ipu-v3: csi: add support for 8 bpp grayscale sensors.
Signed-off-by: Philippe De Muyter Cc: Steve Longerbeam Cc: Boris BREZILLON Cc: Philipp Zabel Cc: Hans Verkuil --- drivers/gpu/ipu-v3/ipu-csi.c |1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/ipu-v3/ipu-csi.c b/drivers/gpu/ipu-v3/ipu-csi.c index 752cdd2..0ab0e3a 100644 --- a/drivers/gpu/ipu-v3/ipu-csi.c +++ b/drivers/gpu/ipu-v3/ipu-csi.c @@ -271,6 +271,7 @@ static int mbus_code_to_bus_cfg(struct ipu_csi_bus_config *cfg, u32 mbus_code) case MEDIA_BUS_FMT_SGBRG8_1X8: case MEDIA_BUS_FMT_SGRBG8_1X8: case MEDIA_BUS_FMT_SRGGB8_1X8: + case MEDIA_BUS_FMT_Y8_1X8: cfg->data_fmt = CSI_SENS_CONF_DATA_FMT_BAYER; cfg->mipi_dt = MIPI_DT_RAW8; cfg->data_width = IPU_CSI_DATA_WIDTH_8; -- 1.7.10.4 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] [media] gpu: ipu-v3: csi: add support for 8 bpp grayscale sensors.
Signed-off-by: Philippe De Muyter <p...@macqel.be> Cc: Steve Longerbeam <slongerb...@gmail.com> Cc: Boris BREZILLON <boris.brezil...@free-electrons.com> Cc: Philipp Zabel <p.za...@pengutronix.de> Cc: Hans Verkuil <hans.verk...@cisco.com> --- drivers/gpu/ipu-v3/ipu-csi.c |1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/ipu-v3/ipu-csi.c b/drivers/gpu/ipu-v3/ipu-csi.c index 752cdd2..0ab0e3a 100644 --- a/drivers/gpu/ipu-v3/ipu-csi.c +++ b/drivers/gpu/ipu-v3/ipu-csi.c @@ -271,6 +271,7 @@ static int mbus_code_to_bus_cfg(struct ipu_csi_bus_config *cfg, u32 mbus_code) case MEDIA_BUS_FMT_SGBRG8_1X8: case MEDIA_BUS_FMT_SGRBG8_1X8: case MEDIA_BUS_FMT_SRGGB8_1X8: + case MEDIA_BUS_FMT_Y8_1X8: cfg->data_fmt = CSI_SENS_CONF_DATA_FMT_BAYER; cfg->mipi_dt = MIPI_DT_RAW8; cfg->data_width = IPU_CSI_DATA_WIDTH_8; -- 1.7.10.4 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 2/2] rtc: add rtc-abx80x, a driver for the Abracon AB x80x i2c rtc
Hi Alexandre,
On Wed, Apr 01, 2015 at 04:52:23AM +0200, Alexandre Belloni wrote:
> From: Philippe De Muyter
>
> This is a basic driver for the ultra-low-power Abracon AB x80x series of RTC
> chips. It supports in particular, the supersets AB0805 and AB1805.
> It allows reading and writing the time, and enables the supercapacitor/
> battery charger.
>
> [a...@arndb.de: abx805 depends on i2c]
> Signed-off-by: Philippe De Muyter
> Cc: Alessandro Zummo
> Signed-off-by: Alexandre Belloni
> Signed-off-by: Arnd Bergmann
> Signed-off-by: Andrew Morton
> ---
...
> +static int abx80x_rtc_read_time(struct device *dev, struct rtc_time *tm)
> +{
> + struct i2c_client *client = to_i2c_client(dev);
> + unsigned char date[8];
> + int err;
> +
> + err = i2c_smbus_read_i2c_block_data(client, ABX8XX_REG_HTH,
> + sizeof(date), date);
> + if (err < 0) {
> + dev_err(>dev, "Unable to read date\n");
> + return -EIO;
> + }
> +
> + tm->tm_sec = bcd2bin(date[ABX8XX_REG_SC] & 0x7F);
> + tm->tm_min = bcd2bin(date[ABX8XX_REG_MN] & 0x7F);
> + tm->tm_hour = bcd2bin(date[ABX8XX_REG_HR] & 0x3F);
> + tm->tm_wday = date[ABX8XX_REG_WD] & 0x7;
> + tm->tm_mday = bcd2bin(date[ABX8XX_REG_DA] & 0x3F);
> + tm->tm_mon = bcd2bin(date[ABX8XX_REG_MO] & 0x1F) - 1;
> + tm->tm_year = bcd2bin(date[ABX8XX_REG_YR]) + 100;
Just a minor nit: I would rather not name the buffer 'date', but 'buf' like
in abx80x_rtc_set_time.
> +
> + err = rtc_valid_tm(tm);
> + if (err < 0)
> + dev_err(>dev, "retrieved date/time is not valid.\n");
> +
> + return err;
> +}
> +
> +static int abx80x_rtc_set_time(struct device *dev, struct rtc_time *tm)
> +{
> + struct i2c_client *client = to_i2c_client(dev);
> + unsigned char buf[8];
> + int err;
> +
...
> +static const struct i2c_device_id abx80x_id[] = {
> + { "abx80x", ABX80X },
should become
{ "abx80x-rtc", ABX80X },
> + { "ab0801", AB0801 },
> + { "ab0803", AB0803 },
> + { "ab0804", AB0804 },
> + { "ab0805", AB0805 },
> + { "ab1801", AB1801 },
> + { "ab1803", AB1803 },
> + { "ab1804", AB1804 },
> + { "ab1805", AB1805 },
> + { }
and likewise
Best regards
Philippe
--
Philippe De Muyter +32 2 6101532 Macq SA rue de l'Aeronef 2 B-1140 Bruxelles
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 1/2] Documentation: bindings: add abracon,abx80x
Hi Alexandre, when I look into Documentation/devicetree/bindings/rtc, it seems that all rtc but a few (mainly old ones) have the "-rtc" suffix in the "compatible" property. Would it not be better to keep it here also, like what I had written ? Best regards Philippe On Wed, Apr 01, 2015 at 04:52:22AM +0200, Alexandre Belloni wrote: > Document the bindings for abracon,abx80x and related compatibles. > > Cc: devicetree-disc...@lists.ozlabs.org > Signed-off-by: Alexandre Belloni > --- > .../devicetree/bindings/rtc/abracon,abx80x.txt | 32 > ++ > 1 file changed, 32 insertions(+) > create mode 100644 Documentation/devicetree/bindings/rtc/abracon,abx80x.txt > > diff --git a/Documentation/devicetree/bindings/rtc/abracon,abx80x.txt > b/Documentation/devicetree/bindings/rtc/abracon,abx80x.txt > new file mode 100644 > index ..6f0d3b2cdd89 > --- /dev/null > +++ b/Documentation/devicetree/bindings/rtc/abracon,abx80x.txt > @@ -0,0 +1,32 @@ > +Abracon ABX80X I2C ultra low power RTC/Alarm chip > + > +The Abracon ABX80X family consist of the ab0801, ab0803, ab0804, ab0805, > ab1801, > +ab1803, ab1804 and ab1805. The ab0805 is the superset of ab080x and the > ab1805 > +is the superset of ab180x. > + > +Required properties: > + > + - "compatible": should one of: > +"abracon,abx80x" > +"abracon,ab0801" > +"abracon,ab0803" > +"abracon,ab0804" > +"abracon,ab0805" > +"abracon,ab1801" > +"abracon,ab1803" > +"abracon,ab1804" > +"abracon,ab1805" > + Using "abracon,abx80x" will enable chip autodetection. > + - "reg": I2C bus address of the device > + > +Optional properties: > + > +The abx804 and abx805 have a trickle charger that is able to charge the > +connected battery or supercap. Both the following properties have to be > defined > +and valid to enable charging: > + > + - "abracon,tc-diode": should be "standard" (0.6V) or "schottky" (0.3V) > + - "abracon,tc-resistor": should be <0>, <3>, <6> or <11>. 0 disables the > output > + resistor, the other values are in ohm. > + > + > -- > 2.1.0 -- Philippe De Muyter +32 2 6101532 Macq SA rue de l'Aeronef 2 B-1140 Bruxelles -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 2/2] rtc: add rtc-abx80x, a driver for the Abracon AB x80x i2c rtc
Hi Alexandre,
On Wed, Apr 01, 2015 at 04:52:23AM +0200, Alexandre Belloni wrote:
From: Philippe De Muyter p...@macqel.be
This is a basic driver for the ultra-low-power Abracon AB x80x series of RTC
chips. It supports in particular, the supersets AB0805 and AB1805.
It allows reading and writing the time, and enables the supercapacitor/
battery charger.
[a...@arndb.de: abx805 depends on i2c]
Signed-off-by: Philippe De Muyter p...@macqel.be
Cc: Alessandro Zummo a.zu...@towertech.it
Signed-off-by: Alexandre Belloni alexandre.bell...@free-electrons.com
Signed-off-by: Arnd Bergmann a...@arndb.de
Signed-off-by: Andrew Morton a...@linux-foundation.org
---
...
+static int abx80x_rtc_read_time(struct device *dev, struct rtc_time *tm)
+{
+ struct i2c_client *client = to_i2c_client(dev);
+ unsigned char date[8];
+ int err;
+
+ err = i2c_smbus_read_i2c_block_data(client, ABX8XX_REG_HTH,
+ sizeof(date), date);
+ if (err 0) {
+ dev_err(client-dev, Unable to read date\n);
+ return -EIO;
+ }
+
+ tm-tm_sec = bcd2bin(date[ABX8XX_REG_SC] 0x7F);
+ tm-tm_min = bcd2bin(date[ABX8XX_REG_MN] 0x7F);
+ tm-tm_hour = bcd2bin(date[ABX8XX_REG_HR] 0x3F);
+ tm-tm_wday = date[ABX8XX_REG_WD] 0x7;
+ tm-tm_mday = bcd2bin(date[ABX8XX_REG_DA] 0x3F);
+ tm-tm_mon = bcd2bin(date[ABX8XX_REG_MO] 0x1F) - 1;
+ tm-tm_year = bcd2bin(date[ABX8XX_REG_YR]) + 100;
Just a minor nit: I would rather not name the buffer 'date', but 'buf' like
in abx80x_rtc_set_time.
+
+ err = rtc_valid_tm(tm);
+ if (err 0)
+ dev_err(client-dev, retrieved date/time is not valid.\n);
+
+ return err;
+}
+
+static int abx80x_rtc_set_time(struct device *dev, struct rtc_time *tm)
+{
+ struct i2c_client *client = to_i2c_client(dev);
+ unsigned char buf[8];
+ int err;
+
...
+static const struct i2c_device_id abx80x_id[] = {
+ { abx80x, ABX80X },
should become
{ abx80x-rtc, ABX80X },
+ { ab0801, AB0801 },
+ { ab0803, AB0803 },
+ { ab0804, AB0804 },
+ { ab0805, AB0805 },
+ { ab1801, AB1801 },
+ { ab1803, AB1803 },
+ { ab1804, AB1804 },
+ { ab1805, AB1805 },
+ { }
and likewise
Best regards
Philippe
--
Philippe De Muyter +32 2 6101532 Macq SA rue de l'Aeronef 2 B-1140 Bruxelles
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 1/2] Documentation: bindings: add abracon,abx80x
Hi Alexandre, when I look into Documentation/devicetree/bindings/rtc, it seems that all rtc but a few (mainly old ones) have the -rtc suffix in the compatible property. Would it not be better to keep it here also, like what I had written ? Best regards Philippe On Wed, Apr 01, 2015 at 04:52:22AM +0200, Alexandre Belloni wrote: Document the bindings for abracon,abx80x and related compatibles. Cc: devicetree-disc...@lists.ozlabs.org Signed-off-by: Alexandre Belloni alexandre.bell...@free-electrons.com --- .../devicetree/bindings/rtc/abracon,abx80x.txt | 32 ++ 1 file changed, 32 insertions(+) create mode 100644 Documentation/devicetree/bindings/rtc/abracon,abx80x.txt diff --git a/Documentation/devicetree/bindings/rtc/abracon,abx80x.txt b/Documentation/devicetree/bindings/rtc/abracon,abx80x.txt new file mode 100644 index ..6f0d3b2cdd89 --- /dev/null +++ b/Documentation/devicetree/bindings/rtc/abracon,abx80x.txt @@ -0,0 +1,32 @@ +Abracon ABX80X I2C ultra low power RTC/Alarm chip + +The Abracon ABX80X family consist of the ab0801, ab0803, ab0804, ab0805, ab1801, +ab1803, ab1804 and ab1805. The ab0805 is the superset of ab080x and the ab1805 +is the superset of ab180x. + +Required properties: + + - compatible: should one of: +abracon,abx80x +abracon,ab0801 +abracon,ab0803 +abracon,ab0804 +abracon,ab0805 +abracon,ab1801 +abracon,ab1803 +abracon,ab1804 +abracon,ab1805 + Using abracon,abx80x will enable chip autodetection. + - reg: I2C bus address of the device + +Optional properties: + +The abx804 and abx805 have a trickle charger that is able to charge the +connected battery or supercap. Both the following properties have to be defined +and valid to enable charging: + + - abracon,tc-diode: should be standard (0.6V) or schottky (0.3V) + - abracon,tc-resistor: should be 0, 3, 6 or 11. 0 disables the output + resistor, the other values are in ohm. + + -- 2.1.0 -- Philippe De Muyter +32 2 6101532 Macq SA rue de l'Aeronef 2 B-1140 Bruxelles -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] rtc: add rtc-abx80x, a driver for the Abracon AB x80x i2c rtc
On Sat, Mar 14, 2015 at 07:09:11PM +0100, Alexandre Belloni wrote: > On 14/03/2015 at 13:44:41 +0100, Philippe De Muyter wrote : > > > + tm->tm_sec = bcd2bin(date[ABX8XX_REG_SC] & 0x7F); > > > + tm->tm_min = bcd2bin(date[ABX8XX_REG_MN] & 0x7F); > > > + tm->tm_hour = bcd2bin(date[ABX8XX_REG_HR] & 0x3F); > > > + tm->tm_wday = date[ABX8XX_REG_WD] & 0x7; > > > + tm->tm_mday = bcd2bin(date[ABX8XX_REG_DA] & 0x3F); > > > + tm->tm_mon = bcd2bin(date[ABX8XX_REG_MO] & 0x1F) - 1; > > > + tm->tm_year = bcd2bin(date[ABX8XX_REG_YR]); > > > + if (tm->tm_year < 70) > > > > Is that still useful for a driver written in 2015 ? > > > > I'd say that this is actually the only correct way to do it. Only dates > before 01/01/1970 00:00 are considered invalid. So, unless adding a > check like: > > if (tm->tm_year < 100) > return -EINVAL; > > in abx80x_rtc_set_time, setting and then reading a date before 2000 will > fail silently. I'm open to add that check. You are right. It is actually more a consistency problem between rtc drivers and thus a question for the rtc-subsytem maintainer. Philippe -- Philippe De Muyter +32 2 6101532 Macq SA rue de l'Aeronef 2 B-1140 Bruxelles -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] rtc: add rtc-abx80x, a driver for the Abracon AB x80x i2c rtc
On Sat, Mar 14, 2015 at 07:09:11PM +0100, Alexandre Belloni wrote: On 14/03/2015 at 13:44:41 +0100, Philippe De Muyter wrote : + tm-tm_sec = bcd2bin(date[ABX8XX_REG_SC] 0x7F); + tm-tm_min = bcd2bin(date[ABX8XX_REG_MN] 0x7F); + tm-tm_hour = bcd2bin(date[ABX8XX_REG_HR] 0x3F); + tm-tm_wday = date[ABX8XX_REG_WD] 0x7; + tm-tm_mday = bcd2bin(date[ABX8XX_REG_DA] 0x3F); + tm-tm_mon = bcd2bin(date[ABX8XX_REG_MO] 0x1F) - 1; + tm-tm_year = bcd2bin(date[ABX8XX_REG_YR]); + if (tm-tm_year 70) Is that still useful for a driver written in 2015 ? I'd say that this is actually the only correct way to do it. Only dates before 01/01/1970 00:00 are considered invalid. So, unless adding a check like: if (tm-tm_year 100) return -EINVAL; in abx80x_rtc_set_time, setting and then reading a date before 2000 will fail silently. I'm open to add that check. You are right. It is actually more a consistency problem between rtc drivers and thus a question for the rtc-subsytem maintainer. Philippe -- Philippe De Muyter +32 2 6101532 Macq SA rue de l'Aeronef 2 B-1140 Bruxelles -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] rtc: add rtc-abx80x, a driver for the Abracon AB x80x i2c rtc
Hi Alexandre,
Thanks for the merge. I have however some comments. They are not meant
to be exhaustive.
Philippe
On Sat, Mar 14, 2015 at 11:05:46AM +0100, Alexandre Belloni wrote:
> From: Philippe De Muyter
>
> This is a basic driver for the ultra-low-power Abracon AB x80x series of RTC
> chips. It supports in particular, the supersets AB0805 and AB1805.
> It allows reading and writing the time, and enables the supercapacitor/
> battery charger.
>
> [a...@arndb.de: abx805 depends on i2c]
> Signed-off-by: Philippe De Muyter
> Cc: Alessandro Zummo
> Signed-off-by: Alexandre Belloni
> Signed-off-by: Arnd Bergmann
> Signed-off-by: Andrew Morton
> ---
> drivers/rtc/Kconfig | 10 ++
> drivers/rtc/Makefile | 1 +
> drivers/rtc/rtc-abx80x.c | 325
> +++
> 3 files changed, 336 insertions(+)
> create mode 100644 drivers/rtc/rtc-abx80x.c
>
> diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
> index b5b5c3d485d6..89507c1858ec 100644
> --- a/drivers/rtc/Kconfig
> +++ b/drivers/rtc/Kconfig
> @@ -164,6 +164,16 @@ config RTC_DRV_ABB5ZES3
> This driver can also be built as a module. If so, the module
> will be called rtc-ab-b5ze-s3.
>
> +config RTC_DRV_ABX80X
> + tristate "Abracon ABx80x"
> + help
> + If you say yes here you get support for Abracon AB080X and AB180X
> + families of ultra-low-power battery- and capacitor-backed real-time
> + clock chips.
> +
> + This driver can also be built as a module. If so, the module
> + will be called rtc-abx80x.
> +
> config RTC_DRV_AS3722
> tristate "ams AS3722 RTC driver"
> depends on MFD_AS3722
> diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile
> index 69c87062b098..7b20b0462cb6 100644
> --- a/drivers/rtc/Makefile
> +++ b/drivers/rtc/Makefile
> @@ -25,6 +25,7 @@ obj-$(CONFIG_RTC_DRV_88PM80X) += rtc-88pm80x.o
> obj-$(CONFIG_RTC_DRV_AB3100) += rtc-ab3100.o
> obj-$(CONFIG_RTC_DRV_AB8500) += rtc-ab8500.o
> obj-$(CONFIG_RTC_DRV_ABB5ZES3) += rtc-ab-b5ze-s3.o
> +obj-$(CONFIG_RTC_DRV_ABX80X) += rtc-abx80x.o
> obj-$(CONFIG_RTC_DRV_ARMADA38X) += rtc-armada38x.o
> obj-$(CONFIG_RTC_DRV_AS3722) += rtc-as3722.o
> obj-$(CONFIG_RTC_DRV_AT32AP700X)+= rtc-at32ap700x.o
> diff --git a/drivers/rtc/rtc-abx80x.c b/drivers/rtc/rtc-abx80x.c
> new file mode 100644
> index ..8424e500f1da
> --- /dev/null
> +++ b/drivers/rtc/rtc-abx80x.c
> @@ -0,0 +1,325 @@
> +/*
> + * A driver for the I2C members of the Abracon AB x8xx RTC family,
> + * and compatible: AB 1805 and AB 0805
> + *
> + * Copyright 2014-2015 Macq S.A.
> + *
> + * Author: Philippe De Muyter
> + * Author: Alexandre Belloni
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +
> +#define ABX8XX_REG_HTH 0x00
> +#define ABX8XX_REG_SC0x01
> +#define ABX8XX_REG_MN0x02
> +#define ABX8XX_REG_HR0x03
> +#define ABX8XX_REG_DA0x04
> +#define ABX8XX_REG_MO0x05
> +#define ABX8XX_REG_YR0x06
> +#define ABX8XX_REG_WD0x07
> +
> +#define ABX8XX_REG_CTRL1 0x10
> +#define ABX8XX_CTRL_WRITEBIT(1)
> +#define ABX8XX_CTRL_12_24BIT(6)
> +
> +#define ABX8XX_REG_CFG_KEY 0x1f
> +#define ABX8XX_CFG_KEY_MISC 0x9d
> +
> +#define ABX8XX_REG_ID0 0x28
> +
> +#define ABX8XX_REG_TRICKLE 0x20
> +#define ABX8XX_TRICKLE_CHARGE_ENABLE 0xa0
> +#define ABX8XX_TRICKLE_STANDARD_DIODE0x8
> +#define ABX8XX_TRICKLE_SCHOTTKY_DIODE0x4
> +
> +static u8 trickle_resistors[] = {0, 3, 6, 11};
> +
> +enum abx80x_chip {AB0801, AB0803, AB0804, AB0805,
> + AB1801, AB1803, AB1804, AB1805, ABX80X};
> +
> +struct abx80x_cap {
> + u16 pn;
> + bool has_tc;
> +};
> +
> +static struct abx80x_cap abx80x_caps[] = {
> + [AB0801] = {.pn = 0x0801},
> + [AB0803] = {.pn = 0x0803},
> + [AB0804] = {.pn = 0x0804, .has_tc = true},
> + [AB0805] = {.pn = 0x0805, .has_tc = true},
> + [AB1801] = {.pn = 0x1801},
> + [AB1803] = {.pn = 0x1803},
> + [AB1804] = {.pn = 0x1804, .has_tc = true},
> + [AB1805] = {.pn = 0x1805, .has_tc = true},
> + [ABX80X] = {.pn = 0}
> +};
> +
> +static struct i2c_driver abx80x_driver;
> +
> +static int abx80x_enable_trickle_charger(struct i2c_client *client,
> +
Re: [PATCH 2/2] rtc: add rtc-abx80x, a driver for the Abracon AB x80x i2c rtc
Hi Alexandre,
Thanks for the merge. I have however some comments. They are not meant
to be exhaustive.
Philippe
On Sat, Mar 14, 2015 at 11:05:46AM +0100, Alexandre Belloni wrote:
From: Philippe De Muyter p...@macqel.be
This is a basic driver for the ultra-low-power Abracon AB x80x series of RTC
chips. It supports in particular, the supersets AB0805 and AB1805.
It allows reading and writing the time, and enables the supercapacitor/
battery charger.
[a...@arndb.de: abx805 depends on i2c]
Signed-off-by: Philippe De Muyter p...@macqel.be
Cc: Alessandro Zummo a.zu...@towertech.it
Signed-off-by: Alexandre Belloni alexandre.bell...@free-electrons.com
Signed-off-by: Arnd Bergmann a...@arndb.de
Signed-off-by: Andrew Morton a...@linux-foundation.org
---
drivers/rtc/Kconfig | 10 ++
drivers/rtc/Makefile | 1 +
drivers/rtc/rtc-abx80x.c | 325
+++
3 files changed, 336 insertions(+)
create mode 100644 drivers/rtc/rtc-abx80x.c
diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index b5b5c3d485d6..89507c1858ec 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -164,6 +164,16 @@ config RTC_DRV_ABB5ZES3
This driver can also be built as a module. If so, the module
will be called rtc-ab-b5ze-s3.
+config RTC_DRV_ABX80X
+ tristate Abracon ABx80x
+ help
+ If you say yes here you get support for Abracon AB080X and AB180X
+ families of ultra-low-power battery- and capacitor-backed real-time
+ clock chips.
+
+ This driver can also be built as a module. If so, the module
+ will be called rtc-abx80x.
+
config RTC_DRV_AS3722
tristate ams AS3722 RTC driver
depends on MFD_AS3722
diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile
index 69c87062b098..7b20b0462cb6 100644
--- a/drivers/rtc/Makefile
+++ b/drivers/rtc/Makefile
@@ -25,6 +25,7 @@ obj-$(CONFIG_RTC_DRV_88PM80X) += rtc-88pm80x.o
obj-$(CONFIG_RTC_DRV_AB3100) += rtc-ab3100.o
obj-$(CONFIG_RTC_DRV_AB8500) += rtc-ab8500.o
obj-$(CONFIG_RTC_DRV_ABB5ZES3) += rtc-ab-b5ze-s3.o
+obj-$(CONFIG_RTC_DRV_ABX80X) += rtc-abx80x.o
obj-$(CONFIG_RTC_DRV_ARMADA38X) += rtc-armada38x.o
obj-$(CONFIG_RTC_DRV_AS3722) += rtc-as3722.o
obj-$(CONFIG_RTC_DRV_AT32AP700X)+= rtc-at32ap700x.o
diff --git a/drivers/rtc/rtc-abx80x.c b/drivers/rtc/rtc-abx80x.c
new file mode 100644
index ..8424e500f1da
--- /dev/null
+++ b/drivers/rtc/rtc-abx80x.c
@@ -0,0 +1,325 @@
+/*
+ * A driver for the I2C members of the Abracon AB x8xx RTC family,
+ * and compatible: AB 1805 and AB 0805
+ *
+ * Copyright 2014-2015 Macq S.A.
+ *
+ * Author: Philippe De Muyter p...@macqel.be
+ * Author: Alexandre Belloni alexandre.bell...@free-electrons.com
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ */
+
+#include linux/bcd.h
+#include linux/i2c.h
+#include linux/module.h
+#include linux/rtc.h
+
+#define ABX8XX_REG_HTH 0x00
+#define ABX8XX_REG_SC0x01
+#define ABX8XX_REG_MN0x02
+#define ABX8XX_REG_HR0x03
+#define ABX8XX_REG_DA0x04
+#define ABX8XX_REG_MO0x05
+#define ABX8XX_REG_YR0x06
+#define ABX8XX_REG_WD0x07
+
+#define ABX8XX_REG_CTRL1 0x10
+#define ABX8XX_CTRL_WRITEBIT(1)
+#define ABX8XX_CTRL_12_24BIT(6)
+
+#define ABX8XX_REG_CFG_KEY 0x1f
+#define ABX8XX_CFG_KEY_MISC 0x9d
+
+#define ABX8XX_REG_ID0 0x28
+
+#define ABX8XX_REG_TRICKLE 0x20
+#define ABX8XX_TRICKLE_CHARGE_ENABLE 0xa0
+#define ABX8XX_TRICKLE_STANDARD_DIODE0x8
+#define ABX8XX_TRICKLE_SCHOTTKY_DIODE0x4
+
+static u8 trickle_resistors[] = {0, 3, 6, 11};
+
+enum abx80x_chip {AB0801, AB0803, AB0804, AB0805,
+ AB1801, AB1803, AB1804, AB1805, ABX80X};
+
+struct abx80x_cap {
+ u16 pn;
+ bool has_tc;
+};
+
+static struct abx80x_cap abx80x_caps[] = {
+ [AB0801] = {.pn = 0x0801},
+ [AB0803] = {.pn = 0x0803},
+ [AB0804] = {.pn = 0x0804, .has_tc = true},
+ [AB0805] = {.pn = 0x0805, .has_tc = true},
+ [AB1801] = {.pn = 0x1801},
+ [AB1803] = {.pn = 0x1803},
+ [AB1804] = {.pn = 0x1804, .has_tc = true},
+ [AB1805] = {.pn = 0x1805, .has_tc = true},
+ [ABX80X] = {.pn = 0}
+};
+
+static struct i2c_driver abx80x_driver;
+
+static int abx80x_enable_trickle_charger(struct i2c_client *client,
+ u8 trickle_cfg)
+{
+ int err;
+
+ /*
+ * Write the configuration key register to enable access to the Trickle
+ * register
+ */
+ err = i2c_smbus_write_byte_data(client, ABX8XX_REG_CFG_KEY,
+ ABX8XX_CFG_KEY_MISC);
+ if (err
Re: [PATCH v2] rtc: add Abracon ABx80x driver
Hi Alexandre,
On Wed, Mar 04, 2015 at 11:38:14AM +0100, Alexandre Belloni wrote:
> On 04/03/2015 at 10:55:56 +0100, Philippe De Muyter wrote :
> > > The "AB08XX Real-Time Clock Family" document states that they are all
> > > software and pin compatible (including the AB18xx).
> >
> > Which part(s) do you really have ? Mine is a 1805. Do all the parts have
> > the trickle charger ? As I don't know, I prefer not to pretend that the
> > driver supports those chips.
> >
>
> I have the AB0805. The trickle charger is not available on ABx8x1 and
> ABx8x3.
>
> > > > My hardware colleagues told me that the only way to enable the 'ultra
> > > > low-power'
> > > > functionality is enabling the trickle charger. And the 'ultra
> > > > low-power'
> > > > functionality is the reason we choose that chip, so I would at least
> > > > keep that as the default behaviour.
> > > >
> > >
> > > My concern is that you have a static configuration. I would expose a
> > > sysfs interface to configure the diode, resistor and enable/disable the
> > > trickle charger. Would that work for you?
> >
> > I think a 'of_xxx' dts/dtb description is the way to go, but I did not want
> > to
> > introduce unnecessaty complexity without knowing other usages. My hardware
> > colleagues followed the recommended design from Abracon.
> >
>
> Yeah, I'm not sure about the DT description, some may argue this is
> configuration. But I guess it actually depends on the battery you are
> using so that could count as hardware description.
As far as I know, it's purely hardware related.
>
> I'll use:
> abracon,tc-diode = "(standard|schottky)"
> abracon,tc-resistor = <(0|3|6|11)>
>
> If both are defined and valid, I'll enable the trickle charger with the
> provided configuration in probe().
My current entry is :
{
...
ab1805@69 {
compatible = "abracon,abx805-rtc";
reg = <0x69>;
position = <3>;
};
};
It would then become :
{
...
ab1805@69 {
compatible = "abracon,abx805-rtc";
reg = <0x69>;
position = <3>;
abracon,tc-diode = "schottky";
abracon,tc-resistor = <3>;
};
};
or if you change the driver's name to "abx80x-rtc"
compatible = "abracon,abx80x-rtc";
That's fine for me. Thanks !
Philippe
--
Philippe De Muyter +32 2 6101532 Macq SA rue de l'Aeronef 2 B-1140 Bruxelles
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] rtc: add Abracon ABx80x driver
Hi Alexandre, On Wed, Mar 04, 2015 at 10:06:01AM +0100, Alexandre Belloni wrote: > On 04/03/2015 at 09:52:42 +0100, Philippe De Muyter wrote : > > > > And is the naming in Philippe's driver appropriate? If it supports the > > > > AB1801 (for example) then why is it described as an "abx805" driver? > > > > > > The real naming is in the form ABx8yz. With: > > > > > > x: 0 or 1, indicate the presence of the reset management > > > y: 0 or 1: 0 is i2c, 1 is spi > > > z: [1-5]: different amount of on chip SRAM. From what I understand, only > > > ABx8y5 are actually recommended for new designs. > > > > My driver is based on the datashet called 'AB18X5 Real-Time Clock with > > Power Management Family', which calls the parts 'AB18X5 family' with X > > being '0' for I2c parts and '1' for SPI parts. > > > > As the part I have and the driver I wrote are I2C, not SPI, I fixed the > > 'X' as 0 in the name of the driver. > > > > The datasheet lists only one part as being 'software and pin compatible' > > with the 'AB1805': the 'AB0805', hence the 'x' in the name I choose : > > abx805. > > > > The "AB08XX Real-Time Clock Family" document states that they are all > software and pin compatible (including the AB18xx). Which part(s) do you really have ? Mine is a 1805. Do all the parts have the trickle charger ? As I don't know, I prefer not to pretend that the driver supports those chips. > > > Actually, my driver is used in production and works fine, because the > > default(reset) value of the 12/24 mode bit is '24 hour mode' and the > > default value of the 'write RTC bit' enables writing. There is > > no real need to play with them. > > > > I know this is unlikely to happen but what if someone messes with the > RTC in the bootloader? Yes, you may unconditionnaly rewrite this register if you want. > > > > > > > What I like in Philippe's driver is the info printed at probe time and > > > the support for trickle charging. However, I wouldn't enable it > > > unconditionally. > > > > My hardware colleagues told me that the only way to enable the 'ultra > > low-power' > > functionality is enabling the trickle charger. And the 'ultra low-power' > > functionality is the reason we choose that chip, so I would at least > > keep that as the default behaviour. > > > > My concern is that you have a static configuration. I would expose a > sysfs interface to configure the diode, resistor and enable/disable the > trickle charger. Would that work for you? I think a 'of_xxx' dts/dtb description is the way to go, but I did not want to introduce unnecessaty complexity without knowing other usages. My hardware colleagues followed the recommended design from Abracon. Philippe -- Philippe De Muyter +32 2 6101532 Macq SA rue de l'Aeronef 2 B-1140 Bruxelles -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] rtc: add Abracon ABx80x driver
On Tue, Mar 03, 2015 at 09:50:32PM +0100, Alexandre Belloni wrote:
> On 03/03/2015 at 12:20:12 -0800, Andrew Morton wrote :
> > On Tue, 3 Mar 2015 02:11:16 +0100 Alexandre Belloni
> > wrote:
> >
> > > On 02/03/2015 at 15:53:37 -0800, Andrew Morton wrote :
> > > > On Sun, 1 Mar 2015 11:27:15 +0100 Alexandre Belloni
> > > > wrote:
> > > >
> > > > > Add support for the i2c RTC from Abracon.
> > > >
> > > > What is the relationship between this patch and
> > > > http://ozlabs.org/~akpm/mmots/broken-out/rtc-add-rtc-abx805-a-driver-for-the-abracon-ab-1805-i2c-rtc.patch?
> > > >
> > >
> > > I'd say drop mine, I couldn't find the other one before writing it...
> > >
> > > I'll try to build on Philippe's driver.
> >
> > Which driver supports the most devices? Your driver has
> >
> > +static const struct i2c_device_id abx80x_id[] = {
> > + { "abx80x", ABX80X },
> > + { "ab0801", AB0801 },
> > + { "ab0802", AB0802 },
> > + { "ab0803", AB0803 },
> > + { "ab0804", AB0804 },
> > + { "ab0805", AB0805 },
> > + { "ab1801", AB1801 },
> > + { "ab1802", AB1802 },
> > + { "ab1803", AB1803 },
> > + { "ab1804", AB1804 },
> > + { "ab1805", AB1805 },
> > + { }
> > +};
> >
> > And Philippe's has
> >
> > +static struct i2c_device_id abx805_id[] = {
> > + { "abx805-rtc", 0 },
The only part my driver is actually tested with, is the 'AB1805', but see
below.
> > + { }
> > +};
> >
> >
> > And is the naming in Philippe's driver appropriate? If it supports the
> > AB1801 (for example) then why is it described as an "abx805" driver?
>
> The real naming is in the form ABx8yz. With:
>
> x: 0 or 1, indicate the presence of the reset management
> y: 0 or 1: 0 is i2c, 1 is spi
> z: [1-5]: different amount of on chip SRAM. From what I understand, only
> ABx8y5 are actually recommended for new designs.
My driver is based on the datashet called 'AB18X5 Real-Time Clock with
Power Management Family', which calls the parts 'AB18X5 family' with X
being '0' for I2c parts and '1' for SPI parts.
As the part I have and the driver I wrote are I2C, not SPI, I fixed the
'X' as 0 in the name of the driver.
The datasheet lists only one part as being 'software and pin compatible'
with the 'AB1805': the 'AB0805', hence the 'x' in the name I choose : abx805.
>
> They all share the same register set, apart from the reset management
> that is only available on AB18yz.
>
> >From my point of view, but I'm obviously biased, I'd take my patch
> because it declares and supports all the available rtc and it also uses
> the i2c_smbus_xxx API that is more robust than the raw i2c_transfer.
I have no opinion on that. The driver I started from uses 'raw i2c_transfer'.
>
> I also take care of the 12/24 mode bit and the write RTC bit which is
> necessary to be able to write to the RTC.
Actually, my driver is used in production and works fine, because the
default(reset) value of the 12/24 mode bit is '24 hour mode' and the
default value of the 'write RTC bit' enables writing. There is
no real need to play with them.
>
> What I like in Philippe's driver is the info printed at probe time and
> the support for trickle charging. However, I wouldn't enable it
> unconditionally.
My hardware colleagues told me that the only way to enable the 'ultra low-power'
functionality is enabling the trickle charger. And the 'ultra low-power'
functionality is the reason we choose that chip, so I would at least
keep that as the default behaviour.
Regards
Philippe
--
Philippe De Muyter +32 2 6101532 Macq SA rue de l'Aeronef 2 B-1140 Bruxelles
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] rtc: add Abracon ABx80x driver
On Tue, Mar 03, 2015 at 09:50:32PM +0100, Alexandre Belloni wrote:
On 03/03/2015 at 12:20:12 -0800, Andrew Morton wrote :
On Tue, 3 Mar 2015 02:11:16 +0100 Alexandre Belloni
alexandre.bell...@free-electrons.com wrote:
On 02/03/2015 at 15:53:37 -0800, Andrew Morton wrote :
On Sun, 1 Mar 2015 11:27:15 +0100 Alexandre Belloni
alexandre.bell...@free-electrons.com wrote:
Add support for the i2c RTC from Abracon.
What is the relationship between this patch and
http://ozlabs.org/~akpm/mmots/broken-out/rtc-add-rtc-abx805-a-driver-for-the-abracon-ab-1805-i2c-rtc.patch?
I'd say drop mine, I couldn't find the other one before writing it...
I'll try to build on Philippe's driver.
Which driver supports the most devices? Your driver has
+static const struct i2c_device_id abx80x_id[] = {
+ { abx80x, ABX80X },
+ { ab0801, AB0801 },
+ { ab0802, AB0802 },
+ { ab0803, AB0803 },
+ { ab0804, AB0804 },
+ { ab0805, AB0805 },
+ { ab1801, AB1801 },
+ { ab1802, AB1802 },
+ { ab1803, AB1803 },
+ { ab1804, AB1804 },
+ { ab1805, AB1805 },
+ { }
+};
And Philippe's has
+static struct i2c_device_id abx805_id[] = {
+ { abx805-rtc, 0 },
The only part my driver is actually tested with, is the 'AB1805', but see
below.
+ { }
+};
And is the naming in Philippe's driver appropriate? If it supports the
AB1801 (for example) then why is it described as an abx805 driver?
The real naming is in the form ABx8yz. With:
x: 0 or 1, indicate the presence of the reset management
y: 0 or 1: 0 is i2c, 1 is spi
z: [1-5]: different amount of on chip SRAM. From what I understand, only
ABx8y5 are actually recommended for new designs.
My driver is based on the datashet called 'AB18X5 Real-Time Clock with
Power Management Family', which calls the parts 'AB18X5 family' with X
being '0' for I2c parts and '1' for SPI parts.
As the part I have and the driver I wrote are I2C, not SPI, I fixed the
'X' as 0 in the name of the driver.
The datasheet lists only one part as being 'software and pin compatible'
with the 'AB1805': the 'AB0805', hence the 'x' in the name I choose : abx805.
They all share the same register set, apart from the reset management
that is only available on AB18yz.
From my point of view, but I'm obviously biased, I'd take my patch
because it declares and supports all the available rtc and it also uses
the i2c_smbus_xxx API that is more robust than the raw i2c_transfer.
I have no opinion on that. The driver I started from uses 'raw i2c_transfer'.
I also take care of the 12/24 mode bit and the write RTC bit which is
necessary to be able to write to the RTC.
Actually, my driver is used in production and works fine, because the
default(reset) value of the 12/24 mode bit is '24 hour mode' and the
default value of the 'write RTC bit' enables writing. There is
no real need to play with them.
What I like in Philippe's driver is the info printed at probe time and
the support for trickle charging. However, I wouldn't enable it
unconditionally.
My hardware colleagues told me that the only way to enable the 'ultra low-power'
functionality is enabling the trickle charger. And the 'ultra low-power'
functionality is the reason we choose that chip, so I would at least
keep that as the default behaviour.
Regards
Philippe
--
Philippe De Muyter +32 2 6101532 Macq SA rue de l'Aeronef 2 B-1140 Bruxelles
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] rtc: add Abracon ABx80x driver
Hi Alexandre, On Wed, Mar 04, 2015 at 10:06:01AM +0100, Alexandre Belloni wrote: On 04/03/2015 at 09:52:42 +0100, Philippe De Muyter wrote : And is the naming in Philippe's driver appropriate? If it supports the AB1801 (for example) then why is it described as an abx805 driver? The real naming is in the form ABx8yz. With: x: 0 or 1, indicate the presence of the reset management y: 0 or 1: 0 is i2c, 1 is spi z: [1-5]: different amount of on chip SRAM. From what I understand, only ABx8y5 are actually recommended for new designs. My driver is based on the datashet called 'AB18X5 Real-Time Clock with Power Management Family', which calls the parts 'AB18X5 family' with X being '0' for I2c parts and '1' for SPI parts. As the part I have and the driver I wrote are I2C, not SPI, I fixed the 'X' as 0 in the name of the driver. The datasheet lists only one part as being 'software and pin compatible' with the 'AB1805': the 'AB0805', hence the 'x' in the name I choose : abx805. The AB08XX Real-Time Clock Family document states that they are all software and pin compatible (including the AB18xx). Which part(s) do you really have ? Mine is a 1805. Do all the parts have the trickle charger ? As I don't know, I prefer not to pretend that the driver supports those chips. Actually, my driver is used in production and works fine, because the default(reset) value of the 12/24 mode bit is '24 hour mode' and the default value of the 'write RTC bit' enables writing. There is no real need to play with them. I know this is unlikely to happen but what if someone messes with the RTC in the bootloader? Yes, you may unconditionnaly rewrite this register if you want. What I like in Philippe's driver is the info printed at probe time and the support for trickle charging. However, I wouldn't enable it unconditionally. My hardware colleagues told me that the only way to enable the 'ultra low-power' functionality is enabling the trickle charger. And the 'ultra low-power' functionality is the reason we choose that chip, so I would at least keep that as the default behaviour. My concern is that you have a static configuration. I would expose a sysfs interface to configure the diode, resistor and enable/disable the trickle charger. Would that work for you? I think a 'of_xxx' dts/dtb description is the way to go, but I did not want to introduce unnecessaty complexity without knowing other usages. My hardware colleagues followed the recommended design from Abracon. Philippe -- Philippe De Muyter +32 2 6101532 Macq SA rue de l'Aeronef 2 B-1140 Bruxelles -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] rtc: add Abracon ABx80x driver
Hi Alexandre,
On Wed, Mar 04, 2015 at 11:38:14AM +0100, Alexandre Belloni wrote:
On 04/03/2015 at 10:55:56 +0100, Philippe De Muyter wrote :
The AB08XX Real-Time Clock Family document states that they are all
software and pin compatible (including the AB18xx).
Which part(s) do you really have ? Mine is a 1805. Do all the parts have
the trickle charger ? As I don't know, I prefer not to pretend that the
driver supports those chips.
I have the AB0805. The trickle charger is not available on ABx8x1 and
ABx8x3.
My hardware colleagues told me that the only way to enable the 'ultra
low-power'
functionality is enabling the trickle charger. And the 'ultra
low-power'
functionality is the reason we choose that chip, so I would at least
keep that as the default behaviour.
My concern is that you have a static configuration. I would expose a
sysfs interface to configure the diode, resistor and enable/disable the
trickle charger. Would that work for you?
I think a 'of_xxx' dts/dtb description is the way to go, but I did not want
to
introduce unnecessaty complexity without knowing other usages. My hardware
colleagues followed the recommended design from Abracon.
Yeah, I'm not sure about the DT description, some may argue this is
configuration. But I guess it actually depends on the battery you are
using so that could count as hardware description.
As far as I know, it's purely hardware related.
I'll use:
abracon,tc-diode = (standard|schottky)
abracon,tc-resistor = (0|3|6|11)
If both are defined and valid, I'll enable the trickle charger with the
provided configuration in probe().
My current entry is :
i2c2 {
...
ab1805@69 {
compatible = abracon,abx805-rtc;
reg = 0x69;
position = 3;
};
};
It would then become :
i2c2 {
...
ab1805@69 {
compatible = abracon,abx805-rtc;
reg = 0x69;
position = 3;
abracon,tc-diode = schottky;
abracon,tc-resistor = 3;
};
};
or if you change the driver's name to abx80x-rtc
compatible = abracon,abx80x-rtc;
That's fine for me. Thanks !
Philippe
--
Philippe De Muyter +32 2 6101532 Macq SA rue de l'Aeronef 2 B-1140 Bruxelles
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [patch] partitions: aix.c: off by one bug
On Tue, Aug 05, 2014 at 11:09:59AM +0300, Dan Carpenter wrote:
> The lvip[] array has "state->limit" elements so the condition here
> should be >= instead of >.
>
> Fixes: 6ceea22bbbc8 ('partitions: add aix lvm partition support files')
> Signed-off-by: Dan Carpenter
>
> diff --git a/block/partitions/aix.c b/block/partitions/aix.c
> index 0a6ed54..f3ed7b2 100644
> --- a/block/partitions/aix.c
> +++ b/block/partitions/aix.c
> @@ -253,7 +253,7 @@ int aix_partition(struct parsed_partitions *state)
> continue;
> }
> lv_ix = be16_to_cpu(p->lv_ix) - 1;
> - if (lv_ix > state->limit) {
> + if (lv_ix >= state->limit) {
> cur_lv_ix = -1;
> continue;
> }
Acked-by: Philippe De Muyter
Thanks
Did you encounter the bug in real life, or only by code review ?
If this has a real risk to happen, a pr_warn would be welcome.
Philippe
--
Philippe De Muyter +32 2 6101532 Macq SA rue de l'Aeronef 2 B-1140 Bruxelles
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [patch] partitions: aix.c: off by one bug
On Tue, Aug 05, 2014 at 11:09:59AM +0300, Dan Carpenter wrote:
The lvip[] array has state-limit elements so the condition here
should be = instead of .
Fixes: 6ceea22bbbc8 ('partitions: add aix lvm partition support files')
Signed-off-by: Dan Carpenter dan.carpen...@oracle.com
diff --git a/block/partitions/aix.c b/block/partitions/aix.c
index 0a6ed54..f3ed7b2 100644
--- a/block/partitions/aix.c
+++ b/block/partitions/aix.c
@@ -253,7 +253,7 @@ int aix_partition(struct parsed_partitions *state)
continue;
}
lv_ix = be16_to_cpu(p-lv_ix) - 1;
- if (lv_ix state-limit) {
+ if (lv_ix = state-limit) {
cur_lv_ix = -1;
continue;
}
Acked-by: Philippe De Muyter p...@macqel.be
Thanks
Did you encounter the bug in real life, or only by code review ?
If this has a real risk to happen, a pr_warn would be welcome.
Philippe
--
Philippe De Muyter +32 2 6101532 Macq SA rue de l'Aeronef 2 B-1140 Bruxelles
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH PING] VFS: mount must return EACCES, not EROFS
On Tue, Jul 08, 2014 at 02:02:18PM -0700, Andrew Morton wrote: > On Thu, 3 Jul 2014 18:29:19 +0200 Philippe De Muyter wrote: > > > On Wed, Jul 02, 2014 at 12:46:51PM -0700, Andrew Morton wrote: > > > On Fri, 27 Jun 2014 10:20:58 +0200 Philippe De Muyter > > > wrote: > > > > > > > Currently, the initial mount of the root file system by the linux > > > > kernel fails with a cryptic message instead of being retried with > > > > the MS_RDONLY flag set, when the device is read-only and the > > > > combination of block driver and filesystem driver yields EROFS. > > > > > > > > I do not know if POSIX mandates that mount(2) must fail with EACCES, nor > > > > if linux aims to strict compliance with POSIX on that point. Consensus > > > > amongst the messages that I have read so far seems to show that linux > > > > kernel hackers feel that EROFS is a more appropriate error code than > > > > EACCES in that case. > > > > > > Isn't the core problem that "the combination of block driver and > > > filesystem driver yields EROFS"? That the fs should instead be > > > returning EACCESS in this case? > > > > Does POSIX or Linux mandate that it should ? > > For info, SCO Unix documents that mount(2) may fail with EROFS : and adds "mount is not part of any currently supported standard" http://osr507doc.sco.com/en/man/html.S/mount.S.html Philippe -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH PING] VFS: mount must return EACCES, not EROFS
On Tue, Jul 08, 2014 at 02:02:18PM -0700, Andrew Morton wrote: On Thu, 3 Jul 2014 18:29:19 +0200 Philippe De Muyter p...@macqel.be wrote: On Wed, Jul 02, 2014 at 12:46:51PM -0700, Andrew Morton wrote: On Fri, 27 Jun 2014 10:20:58 +0200 Philippe De Muyter p...@macqel.be wrote: Currently, the initial mount of the root file system by the linux kernel fails with a cryptic message instead of being retried with the MS_RDONLY flag set, when the device is read-only and the combination of block driver and filesystem driver yields EROFS. I do not know if POSIX mandates that mount(2) must fail with EACCES, nor if linux aims to strict compliance with POSIX on that point. Consensus amongst the messages that I have read so far seems to show that linux kernel hackers feel that EROFS is a more appropriate error code than EACCES in that case. Isn't the core problem that the combination of block driver and filesystem driver yields EROFS? That the fs should instead be returning EACCESS in this case? Does POSIX or Linux mandate that it should ? For info, SCO Unix documents that mount(2) may fail with EROFS : and adds mount is not part of any currently supported standard http://osr507doc.sco.com/en/man/html.S/mount.S.html Philippe -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH PING] VFS: mount must return EACCES, not EROFS
On Wed, Jul 02, 2014 at 12:46:51PM -0700, Andrew Morton wrote:
> On Fri, 27 Jun 2014 10:20:58 +0200 Philippe De Muyter wrote:
>
> > Currently, the initial mount of the root file system by the linux
> > kernel fails with a cryptic message instead of being retried with
> > the MS_RDONLY flag set, when the device is read-only and the
> > combination of block driver and filesystem driver yields EROFS.
> >
> > I do not know if POSIX mandates that mount(2) must fail with EACCES, nor
> > if linux aims to strict compliance with POSIX on that point. Consensus
> > amongst the messages that I have read so far seems to show that linux
> > kernel hackers feel that EROFS is a more appropriate error code than
> > EACCES in that case.
>
> Isn't the core problem that "the combination of block driver and
> filesystem driver yields EROFS"? That the fs should instead be
> returning EACCESS in this case?
Does POSIX or Linux mandate that it should ?
>
> What fs and block driver are we talking about here, anyway?
The problem happened to me with a f2fs filesystem on a sd-card that was
accidentally write-protected and that was put in a SD-card slot (mmc block
driver).
I retested using mount(8) with a similar now intentionnaly write-protected
sd card in a usb reader (usb_storage driver ?) with vfat, f2fs and ext4
filesystems with the following results :
mywdesk:~ # strace -e mount mount /dev/sdb1 /mnt
mount("/dev/sdb1", "/mnt", "vfat", MS_MGC_VAL, NULL) = -1 EROFS (Read-only
file system)
mount: /dev/sdb1 is write-protected, mounting read-only
mount("/dev/sdb1", "/mnt", "vfat", MS_MGC_VAL|MS_RDONLY, NULL) = 0
+++ exited with 0 +++
mywdesk:~ # umount /mnt
mywdesk:~ # strace -e mount mount -t f2fs /dev/sdb2 /mnt
mount("/dev/sdb2", "/mnt", "f2fs", MS_MGC_VAL, NULL) = -1 EROFS (Read-only
file system)
mount: /dev/sdb2 is write-protected, mounting read-only
mount("/dev/sdb2", "/mnt", "f2fs", MS_MGC_VAL|MS_RDONLY, NULL) = 0
+++ exited with 0 +++
mywdesk:~ # umount /mnt
mywdesk:~ # strace -e mount mount /dev/sdb3 /mnt
mount("/dev/sdb3", "/mnt", "ext4", MS_MGC_VAL, NULL) = -1 EROFS (Read-only
file system)
mount: /dev/sdb3 is write-protected, mounting read-only
mount("/dev/sdb3", "/mnt", "ext4", MS_MGC_VAL|MS_RDONLY, NULL) = 0
+++ exited with 0 +++
mywdesk:~ #
All three file-systems (vfat, f2fs & ext4) yield EROFS.
I also quickly grepped for occurences of EROFS under fs/, and found no check
to replace EROFS by EACCES,
while the same grep under drivers/{block,cdrom,ide,md,memstick, mtd,
s390/block,scsi,usb} gives plenty of "return -EROFS;"
So, if no filesystem driver replaces EROFS by EACCES and many block drivers
return EROFS, it seems to me that many combinations will yield EROFS.
> >
> > So, do you choose for my first pragmatic and non-intrusive patch, that
> > lets mount_block_root() retry with MS_RDONLY if the file system
> > returns EROFS (https://lkml.org/lkml/2014/6/18/468) or for the second
> > one that forces all file-systems to return EACCES instead of EROFS.
> > (https://lkml.org/lkml/2014/6/20/98).
>
> They both seem a little hacky to me.
Actually I prefer my first patch, which simply adapts the kernel to the current
situation, like mount(8) already does, instead of trying to impose an ABI
change.
Philippe
--
Philippe De Muyter +32 2 6101532 Macq SA rue de l'Aeronef 2 B-1140 Bruxelles
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH PING] VFS: mount must return EACCES, not EROFS
On Wed, Jul 02, 2014 at 12:46:51PM -0700, Andrew Morton wrote:
On Fri, 27 Jun 2014 10:20:58 +0200 Philippe De Muyter p...@macqel.be wrote:
Currently, the initial mount of the root file system by the linux
kernel fails with a cryptic message instead of being retried with
the MS_RDONLY flag set, when the device is read-only and the
combination of block driver and filesystem driver yields EROFS.
I do not know if POSIX mandates that mount(2) must fail with EACCES, nor
if linux aims to strict compliance with POSIX on that point. Consensus
amongst the messages that I have read so far seems to show that linux
kernel hackers feel that EROFS is a more appropriate error code than
EACCES in that case.
Isn't the core problem that the combination of block driver and
filesystem driver yields EROFS? That the fs should instead be
returning EACCESS in this case?
Does POSIX or Linux mandate that it should ?
What fs and block driver are we talking about here, anyway?
The problem happened to me with a f2fs filesystem on a sd-card that was
accidentally write-protected and that was put in a SD-card slot (mmc block
driver).
I retested using mount(8) with a similar now intentionnaly write-protected
sd card in a usb reader (usb_storage driver ?) with vfat, f2fs and ext4
filesystems with the following results :
mywdesk:~ # strace -e mount mount /dev/sdb1 /mnt
mount(/dev/sdb1, /mnt, vfat, MS_MGC_VAL, NULL) = -1 EROFS (Read-only
file system)
mount: /dev/sdb1 is write-protected, mounting read-only
mount(/dev/sdb1, /mnt, vfat, MS_MGC_VAL|MS_RDONLY, NULL) = 0
+++ exited with 0 +++
mywdesk:~ # umount /mnt
mywdesk:~ # strace -e mount mount -t f2fs /dev/sdb2 /mnt
mount(/dev/sdb2, /mnt, f2fs, MS_MGC_VAL, NULL) = -1 EROFS (Read-only
file system)
mount: /dev/sdb2 is write-protected, mounting read-only
mount(/dev/sdb2, /mnt, f2fs, MS_MGC_VAL|MS_RDONLY, NULL) = 0
+++ exited with 0 +++
mywdesk:~ # umount /mnt
mywdesk:~ # strace -e mount mount /dev/sdb3 /mnt
mount(/dev/sdb3, /mnt, ext4, MS_MGC_VAL, NULL) = -1 EROFS (Read-only
file system)
mount: /dev/sdb3 is write-protected, mounting read-only
mount(/dev/sdb3, /mnt, ext4, MS_MGC_VAL|MS_RDONLY, NULL) = 0
+++ exited with 0 +++
mywdesk:~ #
All three file-systems (vfat, f2fs ext4) yield EROFS.
I also quickly grepped for occurences of EROFS under fs/, and found no check
to replace EROFS by EACCES,
while the same grep under drivers/{block,cdrom,ide,md,memstick, mtd,
s390/block,scsi,usb} gives plenty of return -EROFS;
So, if no filesystem driver replaces EROFS by EACCES and many block drivers
return EROFS, it seems to me that many combinations will yield EROFS.
So, do you choose for my first pragmatic and non-intrusive patch, that
lets mount_block_root() retry with MS_RDONLY if the file system
returns EROFS (https://lkml.org/lkml/2014/6/18/468) or for the second
one that forces all file-systems to return EACCES instead of EROFS.
(https://lkml.org/lkml/2014/6/20/98).
They both seem a little hacky to me.
Actually I prefer my first patch, which simply adapts the kernel to the current
situation, like mount(8) already does, instead of trying to impose an ABI
change.
Philippe
--
Philippe De Muyter +32 2 6101532 Macq SA rue de l'Aeronef 2 B-1140 Bruxelles
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH PING] VFS: mount must return EACCES, not EROFS
PING Currently, the initial mount of the root file system by the linux kernel fails with a cryptic message instead of being retried with the MS_RDONLY flag set, when the device is read-only and the combination of block driver and filesystem driver yields EROFS. I do not know if POSIX mandates that mount(2) must fail with EACCES, nor if linux aims to strict compliance with POSIX on that point. Consensus amongst the messages that I have read so far seems to show that linux kernel hackers feel that EROFS is a more appropriate error code than EACCES in that case. So, do you choose for my first pragmatic and non-intrusive patch, that lets mount_block_root() retry with MS_RDONLY if the file system returns EROFS (https://lkml.org/lkml/2014/6/18/468) or for the second one that forces all file-systems to return EACCES instead of EROFS. (https://lkml.org/lkml/2014/6/20/98). Best regards Philippe On Fri, Jun 20, 2014 at 10:39:22AM +0200, Philippe De Muyter wrote: > mount must return EACCES, not EROFS, when one attempts to mount a > read-only filesystem in read-write mode, but the file-system layer > only transmits the error given by the block layer, and many block > drivers return EROFS in that case, so let's fix it in do_mount. > > Actually it is only a small problem for a user using the mount(1) > command, because EROFS is actually a more explicit answer than > EACCES, but init/do_mounts.c checks only for EACCES, not EROFS, > to decide to retry to mount the root file-system in read-only mode, > and so we are left with an unbootable kernel, and with a cryptic > error message (*) if the root partition happens to be read-only > > (*): VFS: Cannot open root device "mmcblk0p2" or unknown-block(179,2): > error -30 > > Signed-off-by: Philippe De Muyter > Cc: Al Viro > Cc: Dave Chinner > Cc: Andrew Morton > Cc: linux-fsde...@vger.kernel.org > --- > fs/namespace.c |2 ++ > 1 files changed, 2 insertions(+), 0 deletions(-) > > diff --git a/fs/namespace.c b/fs/namespace.c > index 182bc41..6291a3f 100644 > --- a/fs/namespace.c > +++ b/fs/namespace.c > @@ -2411,6 +2411,8 @@ long do_mount(const char *dev_name, const char > *dir_name, > > retval = security_sb_mount(dev_name, , > type_page, flags, data_page); > + if (retval == -EROFS) > + retval = -EACCES; > if (!retval && !may_mount()) > retval = -EPERM; > if (retval) > -- > 1.7.5.3 -- Philippe De Muyter +32 2 6101532 Macq SA rue de l'Aeronef 2 B-1140 Bruxelles -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH PING] VFS: mount must return EACCES, not EROFS
PING Currently, the initial mount of the root file system by the linux kernel fails with a cryptic message instead of being retried with the MS_RDONLY flag set, when the device is read-only and the combination of block driver and filesystem driver yields EROFS. I do not know if POSIX mandates that mount(2) must fail with EACCES, nor if linux aims to strict compliance with POSIX on that point. Consensus amongst the messages that I have read so far seems to show that linux kernel hackers feel that EROFS is a more appropriate error code than EACCES in that case. So, do you choose for my first pragmatic and non-intrusive patch, that lets mount_block_root() retry with MS_RDONLY if the file system returns EROFS (https://lkml.org/lkml/2014/6/18/468) or for the second one that forces all file-systems to return EACCES instead of EROFS. (https://lkml.org/lkml/2014/6/20/98). Best regards Philippe On Fri, Jun 20, 2014 at 10:39:22AM +0200, Philippe De Muyter wrote: mount must return EACCES, not EROFS, when one attempts to mount a read-only filesystem in read-write mode, but the file-system layer only transmits the error given by the block layer, and many block drivers return EROFS in that case, so let's fix it in do_mount. Actually it is only a small problem for a user using the mount(1) command, because EROFS is actually a more explicit answer than EACCES, but init/do_mounts.c checks only for EACCES, not EROFS, to decide to retry to mount the root file-system in read-only mode, and so we are left with an unbootable kernel, and with a cryptic error message (*) if the root partition happens to be read-only (*): VFS: Cannot open root device mmcblk0p2 or unknown-block(179,2): error -30 Signed-off-by: Philippe De Muyter p...@macqel.be Cc: Al Viro v...@zeniv.linux.org.uk Cc: Dave Chinner da...@fromorbit.com Cc: Andrew Morton a...@linux-foundation.org Cc: linux-fsde...@vger.kernel.org --- fs/namespace.c |2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/fs/namespace.c b/fs/namespace.c index 182bc41..6291a3f 100644 --- a/fs/namespace.c +++ b/fs/namespace.c @@ -2411,6 +2411,8 @@ long do_mount(const char *dev_name, const char *dir_name, retval = security_sb_mount(dev_name, path, type_page, flags, data_page); + if (retval == -EROFS) + retval = -EACCES; if (!retval !may_mount()) retval = -EPERM; if (retval) -- 1.7.5.3 -- Philippe De Muyter +32 2 6101532 Macq SA rue de l'Aeronef 2 B-1140 Bruxelles -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] init/do_mounts.c: treat EROFS like EACCES
On Fri, Jun 20, 2014 at 09:09:24AM +1000, Dave Chinner wrote: > On Thu, Jun 19, 2014 at 02:19:50PM -0700, Andrew Morton wrote: > > On Wed, 18 Jun 2014 18:12:44 +0200 Philippe De Muyter > > wrote: > > > > > some combinations of filesystem and block device (at least vfat on mmc) > > > yield -EROFS instead of -EACCES when the device is read-only. Retry > > > mounting with MS_RDONLY set, just like for the EACCES case, instead of > > > failing directly. > > > > > > ... > > > > > > --- a/init/do_mounts.c > > > +++ b/init/do_mounts.c > > > @@ -394,6 +394,7 @@ retry: > > > case 0: > > > goto out; > > > case -EACCES: > > > + case -EROFS: > > > flags |= MS_RDONLY; > > > goto retry; > > > case -EINVAL: > > > > hm, what's going on here. I'd have thought it to be very logical that > > file_system_type.mount() would return EROFS if the device is read-only! > > But I'm suspecting that there is some convention that the fs is > > supposed to return EACCES in this case. So *perhaps* it is vfat-on-mmc > > which needs fixing. Dunno. > > > > Al, are you able to shed light? > > from the mount(2) man page: > > EACCESA component of a path was not searchable. (See also > path_resolution(7).) Or, mounting a read-only filesystem > was attempted without giving the MS_RDONLY flag. Or, the > block device source is located on a filesystem mounted with > the MS_NODEV option. > > So, when the device is read-only, the error should EACCES, not > EROFS. Would seem to me that vfat-on-mmc needs fixing... Looking at the sources of mount(1) https://github.com/karelzak/util-linux/blob/master/sys-utils/mount.c at line 601, we clearly see that mount(1) allows mount(2) to fail with EROFS. We could as well fix the man page of mount(2) Philippe -- Philippe De Muyter +32 2 6101532 Macq SA rue de l'Aeronef 2 B-1140 Bruxelles -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] VFS: mount must return EACCES, not EROFS
mount must return EACCES, not EROFS, when one attempts to mount a read-only filesystem in read-write mode, but the file-system layer only transmits the error given by the block layer, and many block drivers return EROFS in that case, so let's fix it in do_mount. Actually it is only a small problem for a user using the mount(1) command, because EROFS is actually a more explicit answer than EACCES, but init/do_mounts.c checks only for EACCES, not EROFS, to decide to retry to mount the root file-system in read-only mode, and so we are left with an unbootable kernel, and with a cryptic error message (*) if the root partition happens to be read-only (*): VFS: Cannot open root device "mmcblk0p2" or unknown-block(179,2): error -30 Signed-off-by: Philippe De Muyter Cc: Al Viro Cc: Dave Chinner Cc: Andrew Morton Cc: linux-fsde...@vger.kernel.org --- fs/namespace.c |2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/fs/namespace.c b/fs/namespace.c index 182bc41..6291a3f 100644 --- a/fs/namespace.c +++ b/fs/namespace.c @@ -2411,6 +2411,8 @@ long do_mount(const char *dev_name, const char *dir_name, retval = security_sb_mount(dev_name, , type_page, flags, data_page); + if (retval == -EROFS) + retval = -EACCES; if (!retval && !may_mount()) retval = -EPERM; if (retval) -- 1.7.5.3 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] VFS: mount must return EACCES, not EROFS
mount must return EACCES, not EROFS, when one attempts to mount a read-only filesystem in read-write mode, but the file-system layer only transmits the error given by the block layer, and many block drivers return EROFS in that case, so let's fix it in do_mount. Actually it is only a small problem for a user using the mount(1) command, because EROFS is actually a more explicit answer than EACCES, but init/do_mounts.c checks only for EACCES, not EROFS, to decide to retry to mount the root file-system in read-only mode, and so we are left with an unbootable kernel, and with a cryptic error message (*) if the root partition happens to be read-only (*): VFS: Cannot open root device mmcblk0p2 or unknown-block(179,2): error -30 Signed-off-by: Philippe De Muyter p...@macqel.be Cc: Al Viro v...@zeniv.linux.org.uk Cc: Dave Chinner da...@fromorbit.com Cc: Andrew Morton a...@linux-foundation.org Cc: linux-fsde...@vger.kernel.org --- fs/namespace.c |2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/fs/namespace.c b/fs/namespace.c index 182bc41..6291a3f 100644 --- a/fs/namespace.c +++ b/fs/namespace.c @@ -2411,6 +2411,8 @@ long do_mount(const char *dev_name, const char *dir_name, retval = security_sb_mount(dev_name, path, type_page, flags, data_page); + if (retval == -EROFS) + retval = -EACCES; if (!retval !may_mount()) retval = -EPERM; if (retval) -- 1.7.5.3 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] init/do_mounts.c: treat EROFS like EACCES
On Fri, Jun 20, 2014 at 09:09:24AM +1000, Dave Chinner wrote: On Thu, Jun 19, 2014 at 02:19:50PM -0700, Andrew Morton wrote: On Wed, 18 Jun 2014 18:12:44 +0200 Philippe De Muyter p...@macqel.be wrote: some combinations of filesystem and block device (at least vfat on mmc) yield -EROFS instead of -EACCES when the device is read-only. Retry mounting with MS_RDONLY set, just like for the EACCES case, instead of failing directly. ... --- a/init/do_mounts.c +++ b/init/do_mounts.c @@ -394,6 +394,7 @@ retry: case 0: goto out; case -EACCES: + case -EROFS: flags |= MS_RDONLY; goto retry; case -EINVAL: hm, what's going on here. I'd have thought it to be very logical that file_system_type.mount() would return EROFS if the device is read-only! But I'm suspecting that there is some convention that the fs is supposed to return EACCES in this case. So *perhaps* it is vfat-on-mmc which needs fixing. Dunno. Al, are you able to shed light? from the mount(2) man page: EACCESA component of a path was not searchable. (See also path_resolution(7).) Or, mounting a read-only filesystem was attempted without giving the MS_RDONLY flag. Or, the block device source is located on a filesystem mounted with the MS_NODEV option. So, when the device is read-only, the error should EACCES, not EROFS. Would seem to me that vfat-on-mmc needs fixing... Looking at the sources of mount(1) https://github.com/karelzak/util-linux/blob/master/sys-utils/mount.c at line 601, we clearly see that mount(1) allows mount(2) to fail with EROFS. We could as well fix the man page of mount(2) Philippe -- Philippe De Muyter +32 2 6101532 Macq SA rue de l'Aeronef 2 B-1140 Bruxelles -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] init/do_mounts.c: treat EROFS like EACCES
some combinations of filesystem and block device (at least vfat on mmc) yield -EROFS instead of -EACCES when the device is read-only. Retry mounting with MS_RDONLY set, just like for the EACCES case, instead of failing directly. Signed-off-by: Philippe De Muyter Cc: Andrew Morton --- init/do_mounts.c |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/init/do_mounts.c b/init/do_mounts.c index 82f2288..af7b9b8 100644 --- a/init/do_mounts.c +++ b/init/do_mounts.c @@ -394,6 +394,7 @@ retry: case 0: goto out; case -EACCES: + case -EROFS: flags |= MS_RDONLY; goto retry; case -EINVAL: -- 1.7.5.3 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] init/do_mounts.c: treat EROFS like EACCES
some combinations of filesystem and block device (at least vfat on mmc) yield -EROFS instead of -EACCES when the device is read-only. Retry mounting with MS_RDONLY set, just like for the EACCES case, instead of failing directly. Signed-off-by: Philippe De Muyter p...@macqel.be Cc: Andrew Morton a...@linux-foundation.org --- init/do_mounts.c |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/init/do_mounts.c b/init/do_mounts.c index 82f2288..af7b9b8 100644 --- a/init/do_mounts.c +++ b/init/do_mounts.c @@ -394,6 +394,7 @@ retry: case 0: goto out; case -EACCES: + case -EROFS: flags |= MS_RDONLY; goto retry; case -EINVAL: -- 1.7.5.3 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] mmc: sdhci: Remove useless string-split's
Some error or warning messages that appear on one line in the kernel
log are split on two lines in sdhci.c although they are not too long
to fit on one 80-characters line.
This impairs grep'ping for them, so unsplit them.
Signed-off-by: Philippe De Muyter
---
drivers/mmc/host/sdhci.c | 29 ++---
1 files changed, 14 insertions(+), 15 deletions(-)
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index bd8a098..9aa6483 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1226,8 +1226,8 @@ clock_set:
while (!((clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL))
& SDHCI_CLOCK_INT_STABLE)) {
if (timeout == 0) {
- pr_err("%s: Internal clock never "
- "stabilised.\n", mmc_hostname(host->mmc));
+ pr_err("%s: Internal clock never stabilised.\n",
+ mmc_hostname(host->mmc));
sdhci_dumpregs(host);
return;
}
@@ -1993,9 +1993,8 @@ static int sdhci_execute_tuning(struct mmc_host *mmc, u32
opcode)
err = -EIO;
} else {
if (!(ctrl & SDHCI_CTRL_TUNED_CLK)) {
- pr_info(DRIVER_NAME ": Tuning procedure"
- " failed, falling back to fixed sampling"
- " clock\n");
+ pr_info(DRIVER_NAME ": Tuning procedure failed,"
+ " falling back to fixed sampling clock\n");
err = -EIO;
}
}
@@ -2192,8 +2191,8 @@ static void sdhci_timeout_timer(unsigned long data)
spin_lock_irqsave(>lock, flags);
if (host->mrq) {
- pr_err("%s: Timeout waiting for hardware "
- "interrupt.\n", mmc_hostname(host->mmc));
+ pr_err("%s: Timeout waiting for hardware interrupt.\n",
+ mmc_hostname(host->mmc));
sdhci_dumpregs(host);
if (host->data) {
@@ -2825,8 +2824,8 @@ int sdhci_add_host(struct sdhci_host *host)
if (host->flags & (SDHCI_USE_SDMA | SDHCI_USE_ADMA)) {
if (host->ops->enable_dma) {
if (host->ops->enable_dma(host)) {
- pr_warning("%s: No suitable DMA "
- "available. Falling back to PIO.\n",
+ pr_warn("%s: No suitable DMA available."
+ " Falling back to PIO.\n",
mmc_hostname(mmc));
host->flags &=
~(SDHCI_USE_SDMA | SDHCI_USE_ADMA);
@@ -2845,8 +2844,8 @@ int sdhci_add_host(struct sdhci_host *host)
if (!host->adma_desc || !host->align_buffer) {
kfree(host->adma_desc);
kfree(host->align_buffer);
- pr_warning("%s: Unable to allocate ADMA "
- "buffers. Falling back to standard DMA.\n",
+ pr_warn("%s: Unable to allocate ADMA buffers."
+ " Falling back to standard DMA.\n",
mmc_hostname(mmc));
host->flags &= ~SDHCI_USE_ADMA;
}
@@ -3138,8 +3137,8 @@ int sdhci_add_host(struct sdhci_host *host)
mmc->ocr_avail_mmc &= host->ocr_avail_mmc;
if (mmc->ocr_avail == 0) {
- pr_err("%s: Hardware doesn't report any "
- "support voltages.\n", mmc_hostname(mmc));
+ pr_err("%s: Hardware doesn't report any support voltages.\n",
+ mmc_hostname(mmc));
return -ENODEV;
}
@@ -3286,8 +3285,8 @@ void sdhci_remove_host(struct sdhci_host *host, int dead)
host->flags |= SDHCI_DEVICE_DEAD;
if (host->mrq) {
- pr_err("%s: Controller removed during "
- " transfer!\n", mmc_hostname(host->mmc));
+ pr_err("%s: Controller removed during transfer!\n",
+ mmc_hostname(host->mmc));
host->mrq->cmd->error = -ENOMEDIUM;
tasklet_schedule(>finish_tasklet);
--
1.7.5.3
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] mmc: sdhci: Remove useless string-split's
Some error or warning messages that appear on one line in the kernel
log are split on two lines in sdhci.c although they are not too long
to fit on one 80-characters line.
This impairs grep'ping for them, so unsplit them.
Signed-off-by: Philippe De Muyter p...@macqel.be
---
drivers/mmc/host/sdhci.c | 29 ++---
1 files changed, 14 insertions(+), 15 deletions(-)
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index bd8a098..9aa6483 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1226,8 +1226,8 @@ clock_set:
while (!((clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL))
SDHCI_CLOCK_INT_STABLE)) {
if (timeout == 0) {
- pr_err(%s: Internal clock never
- stabilised.\n, mmc_hostname(host-mmc));
+ pr_err(%s: Internal clock never stabilised.\n,
+ mmc_hostname(host-mmc));
sdhci_dumpregs(host);
return;
}
@@ -1993,9 +1993,8 @@ static int sdhci_execute_tuning(struct mmc_host *mmc, u32
opcode)
err = -EIO;
} else {
if (!(ctrl SDHCI_CTRL_TUNED_CLK)) {
- pr_info(DRIVER_NAME : Tuning procedure
-failed, falling back to fixed sampling
-clock\n);
+ pr_info(DRIVER_NAME : Tuning procedure failed,
+falling back to fixed sampling clock\n);
err = -EIO;
}
}
@@ -2192,8 +2191,8 @@ static void sdhci_timeout_timer(unsigned long data)
spin_lock_irqsave(host-lock, flags);
if (host-mrq) {
- pr_err(%s: Timeout waiting for hardware
- interrupt.\n, mmc_hostname(host-mmc));
+ pr_err(%s: Timeout waiting for hardware interrupt.\n,
+ mmc_hostname(host-mmc));
sdhci_dumpregs(host);
if (host-data) {
@@ -2825,8 +2824,8 @@ int sdhci_add_host(struct sdhci_host *host)
if (host-flags (SDHCI_USE_SDMA | SDHCI_USE_ADMA)) {
if (host-ops-enable_dma) {
if (host-ops-enable_dma(host)) {
- pr_warning(%s: No suitable DMA
- available. Falling back to PIO.\n,
+ pr_warn(%s: No suitable DMA available.
+Falling back to PIO.\n,
mmc_hostname(mmc));
host-flags =
~(SDHCI_USE_SDMA | SDHCI_USE_ADMA);
@@ -2845,8 +2844,8 @@ int sdhci_add_host(struct sdhci_host *host)
if (!host-adma_desc || !host-align_buffer) {
kfree(host-adma_desc);
kfree(host-align_buffer);
- pr_warning(%s: Unable to allocate ADMA
- buffers. Falling back to standard DMA.\n,
+ pr_warn(%s: Unable to allocate ADMA buffers.
+Falling back to standard DMA.\n,
mmc_hostname(mmc));
host-flags = ~SDHCI_USE_ADMA;
}
@@ -3138,8 +3137,8 @@ int sdhci_add_host(struct sdhci_host *host)
mmc-ocr_avail_mmc = host-ocr_avail_mmc;
if (mmc-ocr_avail == 0) {
- pr_err(%s: Hardware doesn't report any
- support voltages.\n, mmc_hostname(mmc));
+ pr_err(%s: Hardware doesn't report any support voltages.\n,
+ mmc_hostname(mmc));
return -ENODEV;
}
@@ -3286,8 +3285,8 @@ void sdhci_remove_host(struct sdhci_host *host, int dead)
host-flags |= SDHCI_DEVICE_DEAD;
if (host-mrq) {
- pr_err(%s: Controller removed during
-transfer!\n, mmc_hostname(host-mmc));
+ pr_err(%s: Controller removed during transfer!\n,
+ mmc_hostname(host-mmc));
host-mrq-cmd-error = -ENOMEDIUM;
tasklet_schedule(host-finish_tasklet);
--
1.7.5.3
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
mmc patch needed for 3.13 [was Re: ARM,sdhci_esdhc_imx: INFO: HARDIRQ-safe -> HARDIRQ-unsafe lock order detected]
Hi Chris & Shawn, The patch http://thread.gmane.org/gmane.linux.kernel.mmc/24371, that fixes a bug introduced in 3.13-rc1, is not in 3.13-rc8. Any chance to get it in 3.13 final ? Tested-by: Philippe De Muyter TIA Philippe On Tue, Jan 07, 2014 at 05:05:29PM +0800, Dong Aisheng wrote: > On Tue, Jan 07, 2014 at 10:22:54AM +0100, Philippe De Muyter wrote: > > Hi Shawn & Dong, > > > > On Sat, Dec 14, 2013 at 08:16:15PM +0800, Shawn Guo wrote: > > > On Fri, Dec 13, 2013 at 11:14:25PM +0100, Philippe De Muyter wrote: > > > > Hi Shawn, > > > > > > > > On Fri, Dec 13, 2013 at 09:34:32PM +0800, Shawn Guo wrote: > > > > > Hi Philippe, > > > > > > > > > > On Thu, Dec 12, 2013 at 03:49:25PM +0100, Philippe De Muyter wrote: > > > > > > Hello, > > > > > > > > > > > > I have just booted 3.13-rc3 on my i.MX6DL board, done nothing but > > > > > > watch > > > > > > it start, and I have got this! > > > > > > > > > > There is a patch [1] from Fabio for this issue. > > > > > > > > > > Shawn > > > > > > > > > > [1] http://thread.gmane.org/gmane.linux.kernel.mmc/23786 > > > > > > > > > > > == > > > > > > [ INFO: HARDIRQ-safe -> HARDIRQ-unsafe lock order detected ] > > > > ... > > > > > > > > Thanks. That fixed the problem. Will that go to 3.13 final ? > > > > > > That's a question for Chris. > > > > It seems that it has now been superseded by > > http://thread.gmane.org/gmane.linux.kernel.mmc/24371 > > > > Will those ones go in 3.13-rc8 and final ? > > > > I think patch 1 in that series is better to go in 3.13 final since the issue > exists > since 3.13 rc1. > > Patch 2 in that series is the fix for sdhci-esdhc-imx runtime pm which is > newly added in chris/mmc-next and seems target for 3.14. > > Chris, > Can we make it? > > Regards > Dong Aisheng -- Philippe De Muyter +32 2 6101532 Macq SA rue de l'Aeronef 2 B-1140 Bruxelles -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
mmc patch needed for 3.13 [was Re: ARM,sdhci_esdhc_imx: INFO: HARDIRQ-safe - HARDIRQ-unsafe lock order detected]
Hi Chris Shawn, The patch http://thread.gmane.org/gmane.linux.kernel.mmc/24371, that fixes a bug introduced in 3.13-rc1, is not in 3.13-rc8. Any chance to get it in 3.13 final ? Tested-by: Philippe De Muyter p...@macqel.be TIA Philippe On Tue, Jan 07, 2014 at 05:05:29PM +0800, Dong Aisheng wrote: On Tue, Jan 07, 2014 at 10:22:54AM +0100, Philippe De Muyter wrote: Hi Shawn Dong, On Sat, Dec 14, 2013 at 08:16:15PM +0800, Shawn Guo wrote: On Fri, Dec 13, 2013 at 11:14:25PM +0100, Philippe De Muyter wrote: Hi Shawn, On Fri, Dec 13, 2013 at 09:34:32PM +0800, Shawn Guo wrote: Hi Philippe, On Thu, Dec 12, 2013 at 03:49:25PM +0100, Philippe De Muyter wrote: Hello, I have just booted 3.13-rc3 on my i.MX6DL board, done nothing but watch it start, and I have got this! There is a patch [1] from Fabio for this issue. Shawn [1] http://thread.gmane.org/gmane.linux.kernel.mmc/23786 == [ INFO: HARDIRQ-safe - HARDIRQ-unsafe lock order detected ] ... Thanks. That fixed the problem. Will that go to 3.13 final ? That's a question for Chris. It seems that it has now been superseded by http://thread.gmane.org/gmane.linux.kernel.mmc/24371 Will those ones go in 3.13-rc8 and final ? I think patch 1 in that series is better to go in 3.13 final since the issue exists since 3.13 rc1. Patch 2 in that series is the fix for sdhci-esdhc-imx runtime pm which is newly added in chris/mmc-next and seems target for 3.14. Chris, Can we make it? Regards Dong Aisheng -- Philippe De Muyter +32 2 6101532 Macq SA rue de l'Aeronef 2 B-1140 Bruxelles -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: ARM,sdhci_esdhc_imx: INFO: HARDIRQ-safe -> HARDIRQ-unsafe lock order detected
Hi Shawn & Dong, On Sat, Dec 14, 2013 at 08:16:15PM +0800, Shawn Guo wrote: > On Fri, Dec 13, 2013 at 11:14:25PM +0100, Philippe De Muyter wrote: > > Hi Shawn, > > > > On Fri, Dec 13, 2013 at 09:34:32PM +0800, Shawn Guo wrote: > > > Hi Philippe, > > > > > > On Thu, Dec 12, 2013 at 03:49:25PM +0100, Philippe De Muyter wrote: > > > > Hello, > > > > > > > > I have just booted 3.13-rc3 on my i.MX6DL board, done nothing but watch > > > > it start, and I have got this! > > > > > > There is a patch [1] from Fabio for this issue. > > > > > > Shawn > > > > > > [1] http://thread.gmane.org/gmane.linux.kernel.mmc/23786 > > > > > > > == > > > > [ INFO: HARDIRQ-safe -> HARDIRQ-unsafe lock order detected ] > > ... > > > > Thanks. That fixed the problem. Will that go to 3.13 final ? > > That's a question for Chris. It seems that it has now been superseded by http://thread.gmane.org/gmane.linux.kernel.mmc/24371 Will those ones go in 3.13-rc8 and final ? TIA Philippe -- Philippe De Muyter +32 2 6101532 Macq SA rue de l'Aeronef 2 B-1140 Bruxelles -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: ARM,sdhci_esdhc_imx: INFO: HARDIRQ-safe - HARDIRQ-unsafe lock order detected
Hi Shawn Dong, On Sat, Dec 14, 2013 at 08:16:15PM +0800, Shawn Guo wrote: On Fri, Dec 13, 2013 at 11:14:25PM +0100, Philippe De Muyter wrote: Hi Shawn, On Fri, Dec 13, 2013 at 09:34:32PM +0800, Shawn Guo wrote: Hi Philippe, On Thu, Dec 12, 2013 at 03:49:25PM +0100, Philippe De Muyter wrote: Hello, I have just booted 3.13-rc3 on my i.MX6DL board, done nothing but watch it start, and I have got this! There is a patch [1] from Fabio for this issue. Shawn [1] http://thread.gmane.org/gmane.linux.kernel.mmc/23786 == [ INFO: HARDIRQ-safe - HARDIRQ-unsafe lock order detected ] ... Thanks. That fixed the problem. Will that go to 3.13 final ? That's a question for Chris. It seems that it has now been superseded by http://thread.gmane.org/gmane.linux.kernel.mmc/24371 Will those ones go in 3.13-rc8 and final ? TIA Philippe -- Philippe De Muyter +32 2 6101532 Macq SA rue de l'Aeronef 2 B-1140 Bruxelles -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: ARM,sdhci_esdhc_imx: INFO: HARDIRQ-safe -> HARDIRQ-unsafe lock order detected
Hi Shawn, On Fri, Dec 13, 2013 at 09:34:32PM +0800, Shawn Guo wrote: > Hi Philippe, > > On Thu, Dec 12, 2013 at 03:49:25PM +0100, Philippe De Muyter wrote: > > Hello, > > > > I have just booted 3.13-rc3 on my i.MX6DL board, done nothing but watch > > it start, and I have got this! > > There is a patch [1] from Fabio for this issue. > > Shawn > > [1] http://thread.gmane.org/gmane.linux.kernel.mmc/23786 > > > == > > [ INFO: HARDIRQ-safe -> HARDIRQ-unsafe lock order detected ] ... Thanks. That fixed the problem. Will that go to 3.13 final ? Philippe -- Philippe De Muyter +32 2 6101532 Macq SA rue de l'Aeronef 2 B-1140 Bruxelles -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: ARM,sdhci_esdhc_imx: INFO: HARDIRQ-safe - HARDIRQ-unsafe lock order detected
Hi Shawn, On Fri, Dec 13, 2013 at 09:34:32PM +0800, Shawn Guo wrote: Hi Philippe, On Thu, Dec 12, 2013 at 03:49:25PM +0100, Philippe De Muyter wrote: Hello, I have just booted 3.13-rc3 on my i.MX6DL board, done nothing but watch it start, and I have got this! There is a patch [1] from Fabio for this issue. Shawn [1] http://thread.gmane.org/gmane.linux.kernel.mmc/23786 == [ INFO: HARDIRQ-safe - HARDIRQ-unsafe lock order detected ] ... Thanks. That fixed the problem. Will that go to 3.13 final ? Philippe -- Philippe De Muyter +32 2 6101532 Macq SA rue de l'Aeronef 2 B-1140 Bruxelles -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
ARM,sdhci_esdhc_imx: INFO: HARDIRQ-safe -> HARDIRQ-unsafe lock order detected
c>] kthread+0xcc/0xe8 [<8000e988>] ret_from_fork+0x14/0x2c stack backtrace: CPU: 0 PID: 6 Comm: kworker/u4:0 Not tainted 3.13.0-rc3+ #4 Workqueue: kmmcd mmc_rescan Backtrace: [<80012218>] (dump_backtrace+0x0/0x10c) from [<800123b8>] (show_stack+0x18/0x1c) r6: r5:809c53f0 r4: r3:bf872d00 [<800123a0>] (show_stack+0x0/0x1c) from [<8064a720>] (dump_stack+0x78/0x94) [<8064a6a8>] (dump_stack+0x0/0x94) from [<8005f190>] (check_usage+0x43c/0x5f4) r4:0001 r3:bf872d00 [<8005ed54>] (check_usage+0x0/0x5f4) from [<8005f3a0>] (check_irq_usage+0x58/0xb4) [<8005f348>] (check_irq_usage+0x0/0xb4) from [<80060ce0>] (__lock_acquire+0x1124/0x1cc8) r8:bf8730e8 r7:809c53f0 r6:80e46a54 r5:bf8730d0 r4:0003 [<8005fbbc>] (__lock_acquire+0x0/0x1cc8) from [<80061d9c>] (lock_acquire+0x68/0x7c) [<80061d34>] (lock_acquire+0x0/0x7c) from [<8064f57c>] (mutex_lock_nested+0x5c/0x3b4) r7: r6:80dfc7bc r5:804b55b4 r4:808fd160 [<8064f520>] (mutex_lock_nested+0x0/0x3b4) from [<804b55b4>] (clk_prepare_lock+0x78/0xec) [<804b553c>] (clk_prepare_lock+0x0/0xec) from [<804b5fc8>] (clk_get_rate+0x14/0x64) r6:bf020c40 r5:000186a0 r4:bf828380 r3:80497c54 [<804b5fb4>] (clk_get_rate+0x0/0x64) from [<80497c74>] (esdhc_pltfm_set_clock+0x20/0x2a8) r5:000186a0 r4:bf020c40 [<80497c54>] (esdhc_pltfm_set_clock+0x0/0x2a8) from [<80492b60>] (sdhci_set_clock+0x4c/0x424) [<80492b14>] (sdhci_set_clock+0x0/0x424) from [<804944b8>] (sdhci_do_set_ios+0x314/0x728) [<804941a4>] (sdhci_do_set_ios+0x0/0x728) from [<804948f4>] (sdhci_set_ios+0x28/0x34) [<804948cc>] (sdhci_set_ios+0x0/0x34) from [<80481560>] (mmc_set_bus_mode+0x20/0x24) r5:806947d0 r4:bf020800 [<80481540>] (mmc_set_bus_mode+0x0/0x24) from [<804858d4>] (mmc_attach_mmc+0x140/0x1a0) [<80485794>] (mmc_attach_mmc+0x0/0x1a0) from [<80482348>] (mmc_rescan+0x28c/0x2dc) r5:806947d0 r4:bf020af8 [<804820bc>] (mmc_rescan+0x0/0x2dc) from [<8003df48>] (process_one_work+0x1a4/0x444) r7:bf9bc100 r6:bf80dc00 r5:bf020af8 r4:bf82d880 [<8003dda4>] (process_one_work+0x0/0x444) from [<8003ec7c>] (worker_thread+0x120/0x3e4) [<8003eb5c>] (worker_thread+0x0/0x3e4) from [<8004539c>] (kthread+0xcc/0xe8) [<800452d0>] (kthread+0x0/0xe8) from [<8000e988>] (ret_from_fork+0x14/0x2c) r7: r6: r5:800452d0 r4:bf840580 mmc2: BKOPS_EN bit is not set mmc2: new high speed MMC card at address 0001 -- Philippe De Muyter +32 2 6101532 Macq SA rue de l'Aeronef 2 B-1140 Bruxelles -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
ARM,sdhci_esdhc_imx: INFO: HARDIRQ-safe - HARDIRQ-unsafe lock order detected
) from [8003df48] (process_one_work+0x1a4/0x444) r7:bf9bc100 r6:bf80dc00 r5:bf020af8 r4:bf82d880 [8003dda4] (process_one_work+0x0/0x444) from [8003ec7c] (worker_thread+0x120/0x3e4) [8003eb5c] (worker_thread+0x0/0x3e4) from [8004539c] (kthread+0xcc/0xe8) [800452d0] (kthread+0x0/0xe8) from [8000e988] (ret_from_fork+0x14/0x2c) r7: r6: r5:800452d0 r4:bf840580 mmc2: BKOPS_EN bit is not set mmc2: new high speed MMC card at address 0001 -- Philippe De Muyter +32 2 6101532 Macq SA rue de l'Aeronef 2 B-1140 Bruxelles -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/5] Add aix lvm partitions support files
On Fri, May 24, 2013 at 06:04:52AM -0400, Christoph Hellwig wrote: > On Tue, May 21, 2013 at 09:27:54AM +0200, Philippe De Muyter wrote: > > AIX LVM permits to make "logical volumes" which are made of multiple slices > > of multiple disks. The new code allows only access to the "logical volumes" > > which are made of one slice on the probed disk, a slice being a contiguous > > disk area. The code also detects "logical volumes" made of multiple slices > > on the probed disk, but can not describe them to the partition layer, > > because > > the partition layer generic code does not support that. When such > > non-contiguous "logical volumes" are detected, a diagnostic message is > > printed. > > This very much screams like it should be implemented in the lvm2 > userspace package using the dm kernel driver. Is lvm2 a generic tool for discovering foreign partition layouts, or it is about one linux-specific lvm implementation ? Philippe -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/5] Add aix lvm partitions support files
On Fri, May 24, 2013 at 06:04:52AM -0400, Christoph Hellwig wrote: On Tue, May 21, 2013 at 09:27:54AM +0200, Philippe De Muyter wrote: AIX LVM permits to make logical volumes which are made of multiple slices of multiple disks. The new code allows only access to the logical volumes which are made of one slice on the probed disk, a slice being a contiguous disk area. The code also detects logical volumes made of multiple slices on the probed disk, but can not describe them to the partition layer, because the partition layer generic code does not support that. When such non-contiguous logical volumes are detected, a diagnostic message is printed. This very much screams like it should be implemented in the lvm2 userspace package using the dm kernel driver. Is lvm2 a generic tool for discovering foreign partition layouts, or it is about one linux-specific lvm implementation ? Philippe -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/5] Add aix lvm partitions support files
Hi Andrew, On Mon, May 20, 2013 at 04:39:27PM -0700, Andrew Morton wrote: > On Mon, 29 Apr 2013 23:18:30 +0200 Philippe De Muyter wrote: > > > adding partitions/aix.h and partitions/aix.c > > > > Partitions (called Logical Volumes in AIX) can be non-contiguous or > > even split on more than one disk. Altough we detect such partitions, > > we cannot describe them to the Linux partitions layer, so we simply > > discard them and issue a diagnose message. > > This description is rather hard to follow. > > It appears that the new code permits Linux to access some types of AIX > partitions, but not other types, correct? If so, can we please provide > a more explicit description of which typesw are and are not supported? > AIX LVM permits to make "logical volumes" which are made of multiple slices of multiple disks. The new code allows only access to the "logical volumes" which are made of one slice on the probed disk, a slice being a contiguous disk area. The code also detects "logical volumes" made of multiple slices on the probed disk, but can not describe them to the partition layer, because the partition layer generic code does not support that. When such non-contiguous "logical volumes" are detected, a diagnostic message is printed. Philippe -- Philippe De Muyter +32 2 6101532 Macq SA rue de l'Aeronef 2 B-1140 Bruxelles -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/5] partitions/msdos: enumerate also AIX LVM partitions
Hi Andrew,
On Mon, May 20, 2013 at 04:41:52PM -0700, Andrew Morton wrote:
> On Mon, 29 Apr 2013 23:18:31 +0200 Philippe De Muyter wrote:
>
> > Graft AIX partitions enumeration in partitions/msdos.c
> >
> > There is already a AIX disks detection logic in msdos.c. When an
> > AIX disk has been found, and if configured to, call the aix partitions
> > recognizer. This avoids removal of AIX disks protection from msdos.c,
> > avoids code duplication, and ensures that AIX partitions enumeration
> > is called before plain msdos partitions enumeration.
> >
> > ...
> >
> > --- a/block/partitions/msdos.c
> > +++ b/block/partitions/msdos.c
> > @@ -23,6 +23,7 @@
> > #include "check.h"
> > #include "msdos.h"
> > #include "efi.h"
> > +#include "aix.h"
> >
> > /*
> > * Many architectures don't like unaligned accesses, while
> > @@ -462,8 +463,12 @@ int msdos_partition(struct parsed_partitions *state)
> > */
> > if (aix_magic_present(state, data)) {
> > put_dev_sector(sect);
> > +#ifdef CONFIG_AIX_PARTITION
> > + return aix_partition(state);
> > +#else
> > strlcat(state->pp_buf, " [AIX]", PAGE_SIZE);
> > return 0;
> > +#endif
> > }
> >
> > if (!msdos_magic_present(data + 510)) {
>
> hm, what's going on here.
>
> Why does partitions/msdos.c know about AIX at all? Is there something
> special about AIX partitioning which ties it in with msdos?
Well, PowerPC BIOS (PPCBUG or Open Firmware) mimics PC BIOS and requires the
first block of the disk to describe the boot partition the same way that
PC BIOS does. Remember, the aim of IBM, Motorola and Apple was to replace
the PC's by PowerPC's :). So, an AIX disk can erroneously be recognized as
a dos disk with two partitions (two times the same boot partition). That's
the reason why there is already code in block/partitions/msdos.c to avoid
that.
>
> Now that we have AIX partitioning support, can we simply remove the AIX
> code from msdos.c? So msdos.c will say "I don't know what that is",
> and we fall through to aix.c which says "that's mine!"?
I didn't want to remove the AIX protection from msdos.c, for the likely case
that someone would compile the kernel without AIX_PARTITION support, And, as
a part of the AIX detection was already done by 'aix_magic_present', I did
not want to duplicate it. I can move the AIX detection logic to aix.c and
still keep the AIX protection for msdos.c with some more #ifdef's if you prefer.
Philippe
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/5] partitions/msdos: enumerate also AIX LVM partitions
Hi Andrew,
On Mon, May 20, 2013 at 04:41:52PM -0700, Andrew Morton wrote:
On Mon, 29 Apr 2013 23:18:31 +0200 Philippe De Muyter p...@macqel.be wrote:
Graft AIX partitions enumeration in partitions/msdos.c
There is already a AIX disks detection logic in msdos.c. When an
AIX disk has been found, and if configured to, call the aix partitions
recognizer. This avoids removal of AIX disks protection from msdos.c,
avoids code duplication, and ensures that AIX partitions enumeration
is called before plain msdos partitions enumeration.
...
--- a/block/partitions/msdos.c
+++ b/block/partitions/msdos.c
@@ -23,6 +23,7 @@
#include check.h
#include msdos.h
#include efi.h
+#include aix.h
/*
* Many architectures don't like unaligned accesses, while
@@ -462,8 +463,12 @@ int msdos_partition(struct parsed_partitions *state)
*/
if (aix_magic_present(state, data)) {
put_dev_sector(sect);
+#ifdef CONFIG_AIX_PARTITION
+ return aix_partition(state);
+#else
strlcat(state-pp_buf, [AIX], PAGE_SIZE);
return 0;
+#endif
}
if (!msdos_magic_present(data + 510)) {
hm, what's going on here.
Why does partitions/msdos.c know about AIX at all? Is there something
special about AIX partitioning which ties it in with msdos?
Well, PowerPC BIOS (PPCBUG or Open Firmware) mimics PC BIOS and requires the
first block of the disk to describe the boot partition the same way that
PC BIOS does. Remember, the aim of IBM, Motorola and Apple was to replace
the PC's by PowerPC's :). So, an AIX disk can erroneously be recognized as
a dos disk with two partitions (two times the same boot partition). That's
the reason why there is already code in block/partitions/msdos.c to avoid
that.
Now that we have AIX partitioning support, can we simply remove the AIX
code from msdos.c? So msdos.c will say I don't know what that is,
and we fall through to aix.c which says that's mine!?
I didn't want to remove the AIX protection from msdos.c, for the likely case
that someone would compile the kernel without AIX_PARTITION support, And, as
a part of the AIX detection was already done by 'aix_magic_present', I did
not want to duplicate it. I can move the AIX detection logic to aix.c and
still keep the AIX protection for msdos.c with some more #ifdef's if you prefer.
Philippe
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/5] Add aix lvm partitions support files
Hi Andrew, On Mon, May 20, 2013 at 04:39:27PM -0700, Andrew Morton wrote: On Mon, 29 Apr 2013 23:18:30 +0200 Philippe De Muyter p...@macqel.be wrote: adding partitions/aix.h and partitions/aix.c Partitions (called Logical Volumes in AIX) can be non-contiguous or even split on more than one disk. Altough we detect such partitions, we cannot describe them to the Linux partitions layer, so we simply discard them and issue a diagnose message. This description is rather hard to follow. It appears that the new code permits Linux to access some types of AIX partitions, but not other types, correct? If so, can we please provide a more explicit description of which typesw are and are not supported? AIX LVM permits to make logical volumes which are made of multiple slices of multiple disks. The new code allows only access to the logical volumes which are made of one slice on the probed disk, a slice being a contiguous disk area. The code also detects logical volumes made of multiple slices on the probed disk, but can not describe them to the partition layer, because the partition layer generic code does not support that. When such non-contiguous logical volumes are detected, a diagnostic message is printed. Philippe -- Philippe De Muyter +32 2 6101532 Macq SA rue de l'Aeronef 2 B-1140 Bruxelles -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
AIX LVM support : block/partitions, kpartx, dmraid or lvm2 or ...
Hello, I have recently written a read-only support for linux for AIX 3 & 4 lvm disks, and submitted a patch for the linux block/partitions subsystem, for which I have no feedback yet. Now I wonder, as AIX LVM can describe mirrored and splitted logical volumes, is block/partitions the right place for it ? It seems that only contiguous partitions can be described. Am I mistaken ? I looked also at kpartx, but it seems to me that kpartx has the same limitations as block/partitions. Should I rather make a patch for dmraid or lvm2 or some other tool I am not aware of ? What would be the best place for that support ? I have working code that can generate tables for dmsetup, and I know there is some demand for AIX LVM and AIX JFS support in linux, but where should I send it ? Thanks in advance Philippe PS: Of course, AIX LVM support does only make sense if AIX JFS file systems are supported. I have written read-only support for cwAIX JFSit also, and I plan to submit it when I have a solution for AIX LVM, so that the AIX JFS code can be tested by others. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
AIX LVM support : block/partitions, kpartx, dmraid or lvm2 or ...
Hello, I have recently written a read-only support for linux for AIX 3 4 lvm disks, and submitted a patch for the linux block/partitions subsystem, for which I have no feedback yet. Now I wonder, as AIX LVM can describe mirrored and splitted logical volumes, is block/partitions the right place for it ? It seems that only contiguous partitions can be described. Am I mistaken ? I looked also at kpartx, but it seems to me that kpartx has the same limitations as block/partitions. Should I rather make a patch for dmraid or lvm2 or some other tool I am not aware of ? What would be the best place for that support ? I have working code that can generate tables for dmsetup, and I know there is some demand for AIX LVM and AIX JFS support in linux, but where should I send it ? Thanks in advance Philippe PS: Of course, AIX LVM support does only make sense if AIX JFS file systems are supported. I have written read-only support for cwAIX JFSit also, and I plan to submit it when I have a solution for AIX LVM, so that the AIX JFS code can be tested by others. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 4/5] partitions/Makefile: compile aix.c if configured.
From: Philippe De Muyter Signed-off-by: Philippe De Muyter Cc: Karel Zak Cc: Jens Axboe Cc: Andrew Morton --- block/partitions/Makefile |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/block/partitions/Makefile b/block/partitions/Makefile index 03af8ea..2be4d7b 100644 --- a/block/partitions/Makefile +++ b/block/partitions/Makefile @@ -7,6 +7,7 @@ obj-$(CONFIG_BLOCK) := check.o obj-$(CONFIG_ACORN_PARTITION) += acorn.o obj-$(CONFIG_AMIGA_PARTITION) += amiga.o obj-$(CONFIG_ATARI_PARTITION) += atari.o +obj-$(CONFIG_AIX_PARTITION) += aix.o obj-$(CONFIG_MAC_PARTITION) += mac.o obj-$(CONFIG_LDM_PARTITION) += ldm.o obj-$(CONFIG_MSDOS_PARTITION) += msdos.o -- 1.7.1 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 5/5] partitions/Kconfig: add the AIX_PARTITION entry
From: Philippe De Muyter This is the final patch enabling a user to select AIX lvm partitions detection. Signed-off-by: Philippe De Muyter Cc: Karel Zak Cc: Jens Axboe Cc: Andrew Morton --- block/partitions/Kconfig | 11 +++ 1 files changed, 11 insertions(+), 0 deletions(-) diff --git a/block/partitions/Kconfig b/block/partitions/Kconfig index 75a54e1..4cebb2f 100644 --- a/block/partitions/Kconfig +++ b/block/partitions/Kconfig @@ -68,6 +68,17 @@ config ACORN_PARTITION_RISCIX of machines called RISCiX. If you say 'Y' here, Linux will be able to read disks partitioned under RISCiX. +config AIX_PARTITION + bool "AIX basic partition table support" if PARTITION_ADVANCED + help + Say Y here if you would like to be able to read the hard disk + partition table format used by IBM or Motorola PowerPC machines + running AIX. AIX actually uses a Logical Volume Manager, where + "logical volumes" can be spread across one or multiple disks, + but this driver works only for the simple case of partitions which + are contiguous. + Otherwise, say N. + config OSF_PARTITION bool "Alpha OSF partition support" if PARTITION_ADVANCED default y if ALPHA -- 1.7.1 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 3/5] partitions/msdos: enumerate also AIX LVM partitions
From: Philippe De Muyter
Graft AIX partitions enumeration in partitions/msdos.c
There is already a AIX disks detection logic in msdos.c. When an
AIX disk has been found, and if configured to, call the aix partitions
recognizer. This avoids removal of AIX disks protection from msdos.c,
avoids code duplication, and ensures that AIX partitions enumeration
is called before plain msdos partitions enumeration.
Signed-off-by: Philippe De Muyter
Cc: Karel Zak
Cc: Jens Axboe
Cc: Andrew Morton
---
block/partitions/msdos.c |5 +
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/block/partitions/msdos.c b/block/partitions/msdos.c
index 9bf19e6..9123f25 100644
--- a/block/partitions/msdos.c
+++ b/block/partitions/msdos.c
@@ -23,6 +23,7 @@
#include "check.h"
#include "msdos.h"
#include "efi.h"
+#include "aix.h"
/*
* Many architectures don't like unaligned accesses, while
@@ -462,8 +463,12 @@ int msdos_partition(struct parsed_partitions *state)
*/
if (aix_magic_present(state, data)) {
put_dev_sector(sect);
+#ifdef CONFIG_AIX_PARTITION
+ return aix_partition(state);
+#else
strlcat(state->pp_buf, " [AIX]", PAGE_SIZE);
return 0;
+#endif
}
if (!msdos_magic_present(data + 510)) {
--
1.7.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/5] partitions/msdos.c: end-of-line whitespace and semicolon cleanup
From: Philippe De Muyter
Signed-off-by: Philippe De Muyter
Cc: Karel Zak
Cc: Jens Axboe
Cc: Andrew Morton
---
block/partitions/msdos.c | 12 ++--
1 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/block/partitions/msdos.c b/block/partitions/msdos.c
index 7681cd2..9bf19e6 100644
--- a/block/partitions/msdos.c
+++ b/block/partitions/msdos.c
@@ -90,7 +90,7 @@ static int aix_magic_present(struct parsed_partitions *state,
unsigned char *p)
if (d[0] == '_' && d[1] == 'L' && d[2] == 'V' && d[3] == 'M')
ret = 1;
put_dev_sector(sect);
- };
+ }
return ret;
}
@@ -142,7 +142,7 @@ static void parse_extended(struct parsed_partitions *state,
return;
if (!msdos_magic_present(data + 510))
- goto done;
+ goto done;
p = (struct partition *) (data + 0x1be);
@@ -155,7 +155,7 @@ static void parse_extended(struct parsed_partitions *state,
* and OS/2 seems to use all four entries.
*/
- /*
+ /*
* First process the data partition(s)
*/
for (i=0; i<4; i++, p++) {
@@ -263,7 +263,7 @@ static void parse_solaris_x86(struct parsed_partitions
*state,
}
#if defined(CONFIG_BSD_DISKLABEL)
-/*
+/*
* Create devices for BSD partitions listed in a disklabel, under a
* dos-like partition. See parse_extended() for more information.
*/
@@ -294,7 +294,7 @@ static void parse_bsd(struct parsed_partitions *state,
if (state->next == state->limit)
break;
- if (p->p_fstype == BSD_FS_UNUSED)
+ if (p->p_fstype == BSD_FS_UNUSED)
continue;
bsd_start = le32_to_cpu(p->p_offset);
bsd_size = le32_to_cpu(p->p_size);
@@ -441,7 +441,7 @@ static struct {
{NEW_SOLARIS_X86_PARTITION, parse_solaris_x86},
{0, NULL},
};
-
+
int msdos_partition(struct parsed_partitions *state)
{
sector_t sector_size = bdev_logical_block_size(state->bdev) / 512;
--
1.7.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/5] partitions: Add aix lvm partitions support files
From: Philippe De Muyter
adding partitions/aix.h and partitions/aix.c
Partitions (called Logical Volumes in AIX) can be non-contiguous or
even split on more than one disk. Altough we detect such partitions,
we cannot describe them to the Linux partitions layer, so we simply
discard them and issue a diagnose message.
Signed-off-by: Philippe De Muyter
Cc: Karel Zak
Cc: Jens Axboe
Cc: Andrew Morton
---
block/partitions/aix.c | 291
block/partitions/aix.h |1 +
2 files changed, 292 insertions(+), 0 deletions(-)
create mode 100644 block/partitions/aix.c
create mode 100644 block/partitions/aix.h
diff --git a/block/partitions/aix.c b/block/partitions/aix.c
new file mode 100644
index 000..ef46cf3
--- /dev/null
+++ b/block/partitions/aix.c
@@ -0,0 +1,291 @@
+/*
+ * fs/partitions/aix.c
+ *
+ * Copyright (C) 2012-2013 Philippe De Muyter
+ */
+
+#include "check.h"
+#include "aix.h"
+
+struct lvm_rec {
+ char lvm_id[4]; /* "_LVM" */
+ char reserved4[16];
+ __be32 lvmarea_len;
+ __be32 vgda_len;
+ __be32 vgda_psn[2];
+ char reserved36[10];
+ __be16 pp_size; /* log2(pp_size) */
+ char reserved46[12];
+ __be16 version;
+ };
+
+struct vgda {
+ __be32 secs;
+ __be32 usec;
+ char reserved8[16];
+ __be16 numlvs;
+ __be16 maxlvs;
+ __be16 pp_size;
+ __be16 numpvs;
+ __be16 total_vgdas;
+ __be16 vgda_size;
+ };
+
+struct lvd {
+ __be16 lv_ix;
+ __be16 res2;
+ __be16 res4;
+ __be16 maxsize;
+ __be16 lv_state;
+ __be16 mirror;
+ __be16 mirror_policy;
+ __be16 num_lps;
+ __be16 res10[8];
+ };
+
+struct lvname {
+ char name[64];
+ };
+
+struct ppe {
+ __be16 lv_ix;
+ unsigned short res2;
+ unsigned short res4;
+ __be16 lp_ix;
+ unsigned short res8[12];
+ };
+
+struct pvd {
+ char reserved0[16];
+ __be16 pp_count;
+ char reserved18[2];
+ __be32 psn_part1;
+ char reserved24[8];
+ struct ppe ppe[1016];
+ };
+
+#define LVM_MAXLVS 256
+
+/**
+ * last_lba(): return number of last logical block of device
+ * @bdev: block device
+ *
+ * Description: Returns last LBA value on success, 0 on error.
+ * This is stored (by sd and ide-geometry) in
+ * the part[0] entry for this disk, and is the number of
+ * physical sectors available on the disk.
+ */
+static u64 last_lba(struct block_device *bdev)
+{
+ if (!bdev || !bdev->bd_inode)
+ return 0;
+ return (bdev->bd_inode->i_size >> 9) - 1ULL;
+}
+
+/**
+ * read_lba(): Read bytes from disk, starting at given LBA
+ * @state
+ * @lba
+ * @buffer
+ * @count
+ *
+ * Description: Reads @count bytes from @state->bdev into @buffer.
+ * Returns number of bytes read on success, 0 on error.
+ */
+static size_t read_lba(struct parsed_partitions *state, u64 lba, u8 * buffer,
size_t count)
+{
+ size_t totalreadcount = 0;
+
+ if (!buffer || lba + count / 512 > last_lba(state->bdev))
+return 0;
+
+ while (count) {
+ int copied = 512;
+ Sector sect;
+ unsigned char *data = read_part_sector(state, lba++, );
+ if (!data)
+ break;
+ if (copied > count)
+ copied = count;
+ memcpy(buffer, data, copied);
+ put_dev_sector(sect);
+ buffer += copied;
+ totalreadcount +=copied;
+ count -= copied;
+ }
+ return totalreadcount;
+}
+
+/**
+ * alloc_pvd(): reads physical volume descriptor
+ * @state
+ * @lba
+ *
+ * Description: Returns pvd on success, NULL on error.
+ * Allocates space for pvd and fill it with disk blocks at @lba
+ * Notes: remember to free pvd when you're done!
+ */
+static struct pvd *alloc_pvd(struct parsed_partitions *state, u32 lba)
+{
+ size_t count = sizeof(struct pvd);
+ struct pvd *p;
+
+ p = kmalloc(count, GFP_KERNEL);
+ if (!p)
+ return NULL;
+
+ if (read_lba(state, lba, (u8 *) p, count) < count) {
+ kfree(p);
+ return NULL;
+ }
+ return p;
+}
+
+/**
+ * alloc_lvn(): reads logical volume names
+ * @state
+ * @lba
+ *
+ * Description: Returns lvn on success, NULL on error.
+ * Allocates space for lvn and fill it with disk blocks at @lba
+ * Notes: remember to free lvn when you're done!
+ */
+static struct lvname *alloc_lvn(struct parsed_partitions *state, u32 lba)
+{
+ size_t count = sizeof(struct lvname) * LVM_MAXLVS;
+ struct lvname *p;
+
+ p = kmalloc(count, GFP_KERNEL);
+ if (!p)
+ return NULL;
+
+ if (read_lba(state, lba, (u8 *) p, count) < count) {
+ kfree(p);
+ return N
[PATCH v3 0/5] partitions: Add aix lvm partitions support
This is the version 3 of my patchset to add a basic aix lvm partitions parser to linux. The only modified patch is patch 2. It fixes a problem in the discovering of small (1 pp) partitions in presence of discontiguous partitions. Jens, patch 1 is actually just a cleanup of msdos.c. You could apply it already even if you delay the decision for the other ones, so I could stop resending it here, as patch 3 depends on it. Philippe [PATCH 1/5] partitions/msdos.c: end-of-line whitespace and semicolon cleanup [PATCH 2/5] partitions: Add aix lvm partitions support files [PATCH 3/5] partitions/msdos: enumerate also AIX LVM partitions [PATCH 4/5] partitions/Makefile: compile aix.c if configured. [PATCH 5/5] partitions/Kconfig: add the AIX_PARTITION entry -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v3 0/5] partitions: Add aix lvm partitions support
This is the version 3 of my patchset to add a basic aix lvm partitions parser to linux. The only modified patch is patch 2. It fixes a problem in the discovering of small (1 pp) partitions in presence of discontiguous partitions. Jens, patch 1 is actually just a cleanup of msdos.c. You could apply it already even if you delay the decision for the other ones, so I could stop resending it here, as patch 3 depends on it. Philippe [PATCH 1/5] partitions/msdos.c: end-of-line whitespace and semicolon cleanup [PATCH 2/5] partitions: Add aix lvm partitions support files [PATCH 3/5] partitions/msdos: enumerate also AIX LVM partitions [PATCH 4/5] partitions/Makefile: compile aix.c if configured. [PATCH 5/5] partitions/Kconfig: add the AIX_PARTITION entry -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/5] partitions/msdos.c: end-of-line whitespace and semicolon cleanup
From: Philippe De Muyter p...@macqel.be
Signed-off-by: Philippe De Muyter p...@macqel.be
Cc: Karel Zak k...@redhat.com
Cc: Jens Axboe ax...@kernel.dk
Cc: Andrew Morton a...@linux-foundation.org
---
block/partitions/msdos.c | 12 ++--
1 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/block/partitions/msdos.c b/block/partitions/msdos.c
index 7681cd2..9bf19e6 100644
--- a/block/partitions/msdos.c
+++ b/block/partitions/msdos.c
@@ -90,7 +90,7 @@ static int aix_magic_present(struct parsed_partitions *state,
unsigned char *p)
if (d[0] == '_' d[1] == 'L' d[2] == 'V' d[3] == 'M')
ret = 1;
put_dev_sector(sect);
- };
+ }
return ret;
}
@@ -142,7 +142,7 @@ static void parse_extended(struct parsed_partitions *state,
return;
if (!msdos_magic_present(data + 510))
- goto done;
+ goto done;
p = (struct partition *) (data + 0x1be);
@@ -155,7 +155,7 @@ static void parse_extended(struct parsed_partitions *state,
* and OS/2 seems to use all four entries.
*/
- /*
+ /*
* First process the data partition(s)
*/
for (i=0; i4; i++, p++) {
@@ -263,7 +263,7 @@ static void parse_solaris_x86(struct parsed_partitions
*state,
}
#if defined(CONFIG_BSD_DISKLABEL)
-/*
+/*
* Create devices for BSD partitions listed in a disklabel, under a
* dos-like partition. See parse_extended() for more information.
*/
@@ -294,7 +294,7 @@ static void parse_bsd(struct parsed_partitions *state,
if (state-next == state-limit)
break;
- if (p-p_fstype == BSD_FS_UNUSED)
+ if (p-p_fstype == BSD_FS_UNUSED)
continue;
bsd_start = le32_to_cpu(p-p_offset);
bsd_size = le32_to_cpu(p-p_size);
@@ -441,7 +441,7 @@ static struct {
{NEW_SOLARIS_X86_PARTITION, parse_solaris_x86},
{0, NULL},
};
-
+
int msdos_partition(struct parsed_partitions *state)
{
sector_t sector_size = bdev_logical_block_size(state-bdev) / 512;
--
1.7.1
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/5] partitions: Add aix lvm partitions support files
From: Philippe De Muyter p...@macqel.be
adding partitions/aix.h and partitions/aix.c
Partitions (called Logical Volumes in AIX) can be non-contiguous or
even split on more than one disk. Altough we detect such partitions,
we cannot describe them to the Linux partitions layer, so we simply
discard them and issue a diagnose message.
Signed-off-by: Philippe De Muyter p...@macqel.be
Cc: Karel Zak k...@redhat.com
Cc: Jens Axboe ax...@kernel.dk
Cc: Andrew Morton a...@linux-foundation.org
---
block/partitions/aix.c | 291
block/partitions/aix.h |1 +
2 files changed, 292 insertions(+), 0 deletions(-)
create mode 100644 block/partitions/aix.c
create mode 100644 block/partitions/aix.h
diff --git a/block/partitions/aix.c b/block/partitions/aix.c
new file mode 100644
index 000..ef46cf3
--- /dev/null
+++ b/block/partitions/aix.c
@@ -0,0 +1,291 @@
+/*
+ * fs/partitions/aix.c
+ *
+ * Copyright (C) 2012-2013 Philippe De Muyter p...@macqel.be
+ */
+
+#include check.h
+#include aix.h
+
+struct lvm_rec {
+ char lvm_id[4]; /* _LVM */
+ char reserved4[16];
+ __be32 lvmarea_len;
+ __be32 vgda_len;
+ __be32 vgda_psn[2];
+ char reserved36[10];
+ __be16 pp_size; /* log2(pp_size) */
+ char reserved46[12];
+ __be16 version;
+ };
+
+struct vgda {
+ __be32 secs;
+ __be32 usec;
+ char reserved8[16];
+ __be16 numlvs;
+ __be16 maxlvs;
+ __be16 pp_size;
+ __be16 numpvs;
+ __be16 total_vgdas;
+ __be16 vgda_size;
+ };
+
+struct lvd {
+ __be16 lv_ix;
+ __be16 res2;
+ __be16 res4;
+ __be16 maxsize;
+ __be16 lv_state;
+ __be16 mirror;
+ __be16 mirror_policy;
+ __be16 num_lps;
+ __be16 res10[8];
+ };
+
+struct lvname {
+ char name[64];
+ };
+
+struct ppe {
+ __be16 lv_ix;
+ unsigned short res2;
+ unsigned short res4;
+ __be16 lp_ix;
+ unsigned short res8[12];
+ };
+
+struct pvd {
+ char reserved0[16];
+ __be16 pp_count;
+ char reserved18[2];
+ __be32 psn_part1;
+ char reserved24[8];
+ struct ppe ppe[1016];
+ };
+
+#define LVM_MAXLVS 256
+
+/**
+ * last_lba(): return number of last logical block of device
+ * @bdev: block device
+ *
+ * Description: Returns last LBA value on success, 0 on error.
+ * This is stored (by sd and ide-geometry) in
+ * the part[0] entry for this disk, and is the number of
+ * physical sectors available on the disk.
+ */
+static u64 last_lba(struct block_device *bdev)
+{
+ if (!bdev || !bdev-bd_inode)
+ return 0;
+ return (bdev-bd_inode-i_size 9) - 1ULL;
+}
+
+/**
+ * read_lba(): Read bytes from disk, starting at given LBA
+ * @state
+ * @lba
+ * @buffer
+ * @count
+ *
+ * Description: Reads @count bytes from @state-bdev into @buffer.
+ * Returns number of bytes read on success, 0 on error.
+ */
+static size_t read_lba(struct parsed_partitions *state, u64 lba, u8 * buffer,
size_t count)
+{
+ size_t totalreadcount = 0;
+
+ if (!buffer || lba + count / 512 last_lba(state-bdev))
+return 0;
+
+ while (count) {
+ int copied = 512;
+ Sector sect;
+ unsigned char *data = read_part_sector(state, lba++, sect);
+ if (!data)
+ break;
+ if (copied count)
+ copied = count;
+ memcpy(buffer, data, copied);
+ put_dev_sector(sect);
+ buffer += copied;
+ totalreadcount +=copied;
+ count -= copied;
+ }
+ return totalreadcount;
+}
+
+/**
+ * alloc_pvd(): reads physical volume descriptor
+ * @state
+ * @lba
+ *
+ * Description: Returns pvd on success, NULL on error.
+ * Allocates space for pvd and fill it with disk blocks at @lba
+ * Notes: remember to free pvd when you're done!
+ */
+static struct pvd *alloc_pvd(struct parsed_partitions *state, u32 lba)
+{
+ size_t count = sizeof(struct pvd);
+ struct pvd *p;
+
+ p = kmalloc(count, GFP_KERNEL);
+ if (!p)
+ return NULL;
+
+ if (read_lba(state, lba, (u8 *) p, count) count) {
+ kfree(p);
+ return NULL;
+ }
+ return p;
+}
+
+/**
+ * alloc_lvn(): reads logical volume names
+ * @state
+ * @lba
+ *
+ * Description: Returns lvn on success, NULL on error.
+ * Allocates space for lvn and fill it with disk blocks at @lba
+ * Notes: remember to free lvn when you're done!
+ */
+static struct lvname *alloc_lvn(struct parsed_partitions *state, u32 lba)
+{
+ size_t count = sizeof(struct lvname) * LVM_MAXLVS;
+ struct lvname *p;
+
+ p = kmalloc(count, GFP_KERNEL);
+ if (!p)
+ return NULL;
+
+ if (read_lba(state, lba, (u8 *) p, count) count) {
+ kfree(p
[PATCH 3/5] partitions/msdos: enumerate also AIX LVM partitions
From: Philippe De Muyter p...@macqel.be
Graft AIX partitions enumeration in partitions/msdos.c
There is already a AIX disks detection logic in msdos.c. When an
AIX disk has been found, and if configured to, call the aix partitions
recognizer. This avoids removal of AIX disks protection from msdos.c,
avoids code duplication, and ensures that AIX partitions enumeration
is called before plain msdos partitions enumeration.
Signed-off-by: Philippe De Muyter p...@macqel.be
Cc: Karel Zak k...@redhat.com
Cc: Jens Axboe ax...@kernel.dk
Cc: Andrew Morton a...@linux-foundation.org
---
block/partitions/msdos.c |5 +
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/block/partitions/msdos.c b/block/partitions/msdos.c
index 9bf19e6..9123f25 100644
--- a/block/partitions/msdos.c
+++ b/block/partitions/msdos.c
@@ -23,6 +23,7 @@
#include check.h
#include msdos.h
#include efi.h
+#include aix.h
/*
* Many architectures don't like unaligned accesses, while
@@ -462,8 +463,12 @@ int msdos_partition(struct parsed_partitions *state)
*/
if (aix_magic_present(state, data)) {
put_dev_sector(sect);
+#ifdef CONFIG_AIX_PARTITION
+ return aix_partition(state);
+#else
strlcat(state-pp_buf, [AIX], PAGE_SIZE);
return 0;
+#endif
}
if (!msdos_magic_present(data + 510)) {
--
1.7.1
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 5/5] partitions/Kconfig: add the AIX_PARTITION entry
From: Philippe De Muyter p...@macqel.be This is the final patch enabling a user to select AIX lvm partitions detection. Signed-off-by: Philippe De Muyter p...@macqel.be Cc: Karel Zak k...@redhat.com Cc: Jens Axboe ax...@kernel.dk Cc: Andrew Morton a...@linux-foundation.org --- block/partitions/Kconfig | 11 +++ 1 files changed, 11 insertions(+), 0 deletions(-) diff --git a/block/partitions/Kconfig b/block/partitions/Kconfig index 75a54e1..4cebb2f 100644 --- a/block/partitions/Kconfig +++ b/block/partitions/Kconfig @@ -68,6 +68,17 @@ config ACORN_PARTITION_RISCIX of machines called RISCiX. If you say 'Y' here, Linux will be able to read disks partitioned under RISCiX. +config AIX_PARTITION + bool AIX basic partition table support if PARTITION_ADVANCED + help + Say Y here if you would like to be able to read the hard disk + partition table format used by IBM or Motorola PowerPC machines + running AIX. AIX actually uses a Logical Volume Manager, where + logical volumes can be spread across one or multiple disks, + but this driver works only for the simple case of partitions which + are contiguous. + Otherwise, say N. + config OSF_PARTITION bool Alpha OSF partition support if PARTITION_ADVANCED default y if ALPHA -- 1.7.1 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 4/5] partitions/Makefile: compile aix.c if configured.
From: Philippe De Muyter p...@macqel.be Signed-off-by: Philippe De Muyter p...@macqel.be Cc: Karel Zak k...@redhat.com Cc: Jens Axboe ax...@kernel.dk Cc: Andrew Morton a...@linux-foundation.org --- block/partitions/Makefile |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/block/partitions/Makefile b/block/partitions/Makefile index 03af8ea..2be4d7b 100644 --- a/block/partitions/Makefile +++ b/block/partitions/Makefile @@ -7,6 +7,7 @@ obj-$(CONFIG_BLOCK) := check.o obj-$(CONFIG_ACORN_PARTITION) += acorn.o obj-$(CONFIG_AMIGA_PARTITION) += amiga.o obj-$(CONFIG_ATARI_PARTITION) += atari.o +obj-$(CONFIG_AIX_PARTITION) += aix.o obj-$(CONFIG_MAC_PARTITION) += mac.o obj-$(CONFIG_LDM_PARTITION) += ldm.o obj-$(CONFIG_MSDOS_PARTITION) += msdos.o -- 1.7.1 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/5] Add aix lvm partitions support files
On Tue, Apr 30, 2013 at 09:18:40AM +0200, Philippe De Muyter wrote: > On Tue, Apr 30, 2013 at 09:08:40AM +0200, Philippe De Muyter wrote: > > On Mon, Apr 29, 2013 at 11:50:51PM +0200, Philippe De Muyter wrote: > > > On Mon, Apr 29, 2013 at 01:40:41PM +0200, Philippe De Muyter wrote: > > > > On Mon, Apr 29, 2013 at 11:37:56AM +0200, Karel Zak wrote: > > > > > > > > > > Philippe, do you have any disk image with AIX LVM? It would be nice to > > > > > have a way how to test the code. I'd like to add support for AIX to > > > > > libblkid too. > > > > > > > > Of course. But that's not a couple of blocks. I'll try to cut the > > > > slice that you need. > > > > > > Here is one example. I'll try to send another one tomorrow. > > > > Here is the other example. > > And here is a third one And now a fourth one, this one with partitions that are not contiguous. Philippe -- Philippe De Muyter +32 2 6101532 Macq SA rue de l'Aeronef 2 B-1140 Bruxelles aixlvmhdr-136g.bz2 Description: application/bzip
Re: [PATCH 2/5] Add aix lvm partitions support files
On Tue, Apr 30, 2013 at 09:08:40AM +0200, Philippe De Muyter wrote: > On Mon, Apr 29, 2013 at 11:50:51PM +0200, Philippe De Muyter wrote: > > On Mon, Apr 29, 2013 at 01:40:41PM +0200, Philippe De Muyter wrote: > > > On Mon, Apr 29, 2013 at 11:37:56AM +0200, Karel Zak wrote: > > > > > > > > Philippe, do you have any disk image with AIX LVM? It would be nice to > > > > have a way how to test the code. I'd like to add support for AIX to > > > > libblkid too. > > > > > > Of course. But that's not a couple of blocks. I'll try to cut the > > > slice that you need. > > > > Here is one example. I'll try to send another one tomorrow. > > Here is the other example. And here is a third one Philippe -- Philippe De Muyter +32 2 6101532 Macq SA rue de l'Aeronef 2 B-1140 Bruxelles aixlvmhdr-jori.bz2 Description: application/bzip
Re: [PATCH 2/5] Add aix lvm partitions support files
On Mon, Apr 29, 2013 at 11:50:51PM +0200, Philippe De Muyter wrote: > On Mon, Apr 29, 2013 at 01:40:41PM +0200, Philippe De Muyter wrote: > > On Mon, Apr 29, 2013 at 11:37:56AM +0200, Karel Zak wrote: > > > > > > Philippe, do you have any disk image with AIX LVM? It would be nice to > > > have a way how to test the code. I'd like to add support for AIX to > > > libblkid too. > > > > Of course. But that's not a couple of blocks. I'll try to cut the > > slice that you need. > > Here is one example. I'll try to send another one tomorrow. Here is the other example. Philippe -- Philippe De Muyter +32 2 6101532 Macq SA rue de l'Aeronef 2 B-1140 Bruxelles aixlvmhdr-goofy.bz2 Description: application/bzip
Re: [PATCH 2/5] Add aix lvm partitions support files
On Tue, Apr 30, 2013 at 08:41:52AM +0200, Jens Axboe wrote: > On Mon, Apr 29 2013, Philippe De Muyter wrote: > > > > so sda is 8,0 and sdb is 8,16 > > > > and if, while discovering partitions of /dev/sda, I try to make a > > partition 16 or higher, it is silently discarded by 'put_partition'. > > > > Is that changed ? > > That's a set limitation of sd, it does not apply to other devices. The > legacy IDE code used 64 for max partitions, for instance. So Karel is > right, you should not make any assumptions about the max number of > partitions, it is driver dependent. I replaced that by state->limit in the v2 series. Philippe -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/5] Add aix lvm partitions support files
On Tue, Apr 30, 2013 at 08:41:52AM +0200, Jens Axboe wrote: On Mon, Apr 29 2013, Philippe De Muyter wrote: so sda is 8,0 and sdb is 8,16 and if, while discovering partitions of /dev/sda, I try to make a partition 16 or higher, it is silently discarded by 'put_partition'. Is that changed ? That's a set limitation of sd, it does not apply to other devices. The legacy IDE code used 64 for max partitions, for instance. So Karel is right, you should not make any assumptions about the max number of partitions, it is driver dependent. I replaced that by state-limit in the v2 series. Philippe -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/5] Add aix lvm partitions support files
On Mon, Apr 29, 2013 at 11:50:51PM +0200, Philippe De Muyter wrote: On Mon, Apr 29, 2013 at 01:40:41PM +0200, Philippe De Muyter wrote: On Mon, Apr 29, 2013 at 11:37:56AM +0200, Karel Zak wrote: Philippe, do you have any disk image with AIX LVM? It would be nice to have a way how to test the code. I'd like to add support for AIX to libblkid too. Of course. But that's not a couple of blocks. I'll try to cut the slice that you need. Here is one example. I'll try to send another one tomorrow. Here is the other example. Philippe -- Philippe De Muyter +32 2 6101532 Macq SA rue de l'Aeronef 2 B-1140 Bruxelles aixlvmhdr-goofy.bz2 Description: application/bzip
Re: [PATCH 2/5] Add aix lvm partitions support files
On Tue, Apr 30, 2013 at 09:08:40AM +0200, Philippe De Muyter wrote: On Mon, Apr 29, 2013 at 11:50:51PM +0200, Philippe De Muyter wrote: On Mon, Apr 29, 2013 at 01:40:41PM +0200, Philippe De Muyter wrote: On Mon, Apr 29, 2013 at 11:37:56AM +0200, Karel Zak wrote: Philippe, do you have any disk image with AIX LVM? It would be nice to have a way how to test the code. I'd like to add support for AIX to libblkid too. Of course. But that's not a couple of blocks. I'll try to cut the slice that you need. Here is one example. I'll try to send another one tomorrow. Here is the other example. And here is a third one Philippe -- Philippe De Muyter +32 2 6101532 Macq SA rue de l'Aeronef 2 B-1140 Bruxelles aixlvmhdr-jori.bz2 Description: application/bzip
Re: [PATCH 2/5] Add aix lvm partitions support files
On Tue, Apr 30, 2013 at 09:18:40AM +0200, Philippe De Muyter wrote: On Tue, Apr 30, 2013 at 09:08:40AM +0200, Philippe De Muyter wrote: On Mon, Apr 29, 2013 at 11:50:51PM +0200, Philippe De Muyter wrote: On Mon, Apr 29, 2013 at 01:40:41PM +0200, Philippe De Muyter wrote: On Mon, Apr 29, 2013 at 11:37:56AM +0200, Karel Zak wrote: Philippe, do you have any disk image with AIX LVM? It would be nice to have a way how to test the code. I'd like to add support for AIX to libblkid too. Of course. But that's not a couple of blocks. I'll try to cut the slice that you need. Here is one example. I'll try to send another one tomorrow. Here is the other example. And here is a third one And now a fourth one, this one with partitions that are not contiguous. Philippe -- Philippe De Muyter +32 2 6101532 Macq SA rue de l'Aeronef 2 B-1140 Bruxelles aixlvmhdr-136g.bz2 Description: application/bzip
Re: [PATCH 2/5] Add aix lvm partitions support files
On Mon, Apr 29, 2013 at 01:40:41PM +0200, Philippe De Muyter wrote: > On Mon, Apr 29, 2013 at 11:37:56AM +0200, Karel Zak wrote: > > > > Philippe, do you have any disk image with AIX LVM? It would be nice to > > have a way how to test the code. I'd like to add support for AIX to > > libblkid too. > > Of course. But that's not a couple of blocks. I'll try to cut the > slice that you need. Here is one example. I'll try to send another one tomorrow. Philippe aixlvmhdr-128g.bz2 Description: application/bzip
[PATCH 2/5] Add aix lvm partitions support files
adding partitions/aix.h and partitions/aix.c
Partitions (called Logical Volumes in AIX) can be non-contiguous or
even split on more than one disk. Altough we detect such partitions,
we cannot describe them to the Linux partitions layer, so we simply
discard them and issue a diagnose message.
Signed-off-by: Philippe De Muyter
Cc: Karel Zak
Cc: Jens Axboe
Cc: Andrew Morton
---
block/partitions/aix.c | 290
block/partitions/aix.h |1 +
2 files changed, 291 insertions(+), 0 deletions(-)
create mode 100644 block/partitions/aix.c
create mode 100644 block/partitions/aix.h
diff --git a/block/partitions/aix.c b/block/partitions/aix.c
new file mode 100644
index 000..b3288d2
--- /dev/null
+++ b/block/partitions/aix.c
@@ -0,0 +1,290 @@
+/*
+ * fs/partitions/aix.c
+ *
+ * Copyright (C) 2012-2013 Philippe De Muyter
+ */
+
+#include "check.h"
+#include "aix.h"
+
+struct lvm_rec {
+ char lvm_id[4]; /* "_LVM" */
+ char reserved4[16];
+ __be32 lvmarea_len;
+ __be32 vgda_len;
+ __be32 vgda_psn[2];
+ char reserved36[10];
+ __be16 pp_size; /* log2(pp_size) */
+ char reserved46[12];
+ __be16 version;
+ };
+
+struct vgda {
+ __be32 secs;
+ __be32 usec;
+ char reserved8[16];
+ __be16 numlvs;
+ __be16 maxlvs;
+ __be16 pp_size;
+ __be16 numpvs;
+ __be16 total_vgdas;
+ __be16 vgda_size;
+ };
+
+struct lvd {
+ __be16 lv_ix;
+ __be16 res2;
+ __be16 res4;
+ __be16 maxsize;
+ __be16 lv_state;
+ __be16 mirror;
+ __be16 mirror_policy;
+ __be16 num_lps;
+ __be16 res10[8];
+ };
+
+struct lvname {
+ char name[64];
+ };
+
+struct ppe {
+ __be16 lv_ix;
+ unsigned short res2;
+ unsigned short res4;
+ __be16 lp_ix;
+ unsigned short res8[12];
+ };
+
+struct pvd {
+ char reserved0[16];
+ __be16 pp_count;
+ char reserved18[2];
+ __be32 psn_part1;
+ char reserved24[8];
+ struct ppe ppe[1016];
+ };
+
+#define LVM_MAXLVS 256
+
+/**
+ * last_lba(): return number of last logical block of device
+ * @bdev: block device
+ *
+ * Description: Returns last LBA value on success, 0 on error.
+ * This is stored (by sd and ide-geometry) in
+ * the part[0] entry for this disk, and is the number of
+ * physical sectors available on the disk.
+ */
+static u64 last_lba(struct block_device *bdev)
+{
+ if (!bdev || !bdev->bd_inode)
+ return 0;
+ return (bdev->bd_inode->i_size >> 9) - 1ULL;
+}
+
+/**
+ * read_lba(): Read bytes from disk, starting at given LBA
+ * @state
+ * @lba
+ * @buffer
+ * @count
+ *
+ * Description: Reads @count bytes from @state->bdev into @buffer.
+ * Returns number of bytes read on success, 0 on error.
+ */
+static size_t read_lba(struct parsed_partitions *state, u64 lba, u8 * buffer,
size_t count)
+{
+ size_t totalreadcount = 0;
+
+ if (!buffer || lba + count / 512 > last_lba(state->bdev))
+return 0;
+
+ while (count) {
+ int copied = 512;
+ Sector sect;
+ unsigned char *data = read_part_sector(state, lba++, );
+ if (!data)
+ break;
+ if (copied > count)
+ copied = count;
+ memcpy(buffer, data, copied);
+ put_dev_sector(sect);
+ buffer += copied;
+ totalreadcount +=copied;
+ count -= copied;
+ }
+ return totalreadcount;
+}
+
+/**
+ * alloc_pvd(): reads physical volume descriptor
+ * @state
+ * @lba
+ *
+ * Description: Returns pvd on success, NULL on error.
+ * Allocates space for pvd and fill it with disk blocks at @lba
+ * Notes: remember to free pvd when you're done!
+ */
+static struct pvd *alloc_pvd(struct parsed_partitions *state, u32 lba)
+{
+ size_t count = sizeof(struct pvd);
+ struct pvd *p;
+
+ p = kmalloc(count, GFP_KERNEL);
+ if (!p)
+ return NULL;
+
+ if (read_lba(state, lba, (u8 *) p, count) < count) {
+ kfree(p);
+ return NULL;
+ }
+ return p;
+}
+
+/**
+ * alloc_lvn(): reads logical volume names
+ * @state
+ * @lba
+ *
+ * Description: Returns lvn on success, NULL on error.
+ * Allocates space for lvn and fill it with disk blocks at @lba
+ * Notes: remember to free lvn when you're done!
+ */
+static struct lvname *alloc_lvn(struct parsed_partitions *state, u32 lba)
+{
+ size_t count = sizeof(struct lvname) * LVM_MAXLVS;
+ struct lvname *p;
+
+ p = kmalloc(count, GFP_KERNEL);
+ if (!p)
+ return NULL;
+
+ if (read_lba(state, lba, (u8 *) p, count) < count) {
+ kfree(p);
+ return NULL;
+ }
+ return p;
+}
+
+in
[PATCH v2 0/5] partitions: add AIX LVM support
This is a revision of the patch serie I sent some days ago, with fixes based on observations by Karel Zak, and other cleanups. This enables finding the contiguous partitions in AIX LVM disks. Patch 1 is actually just a cleanup of msdos.c, but as patch 3 depends on it I prefer to put it here. Philippe [PATCH 1/5] partitions/msdos.c: end-of-line whitespace and semicolon cleanup [PATCH 2/5] Add aix lvm partitions support files [PATCH 3/5] partitions/msdos: enumerate also AIX LVM partitions [PATCH 4/5] partitions/Makefile: compile aix.c if configured. [PATCH 5/5] partitions/Kconfig: add the AIX_PARTITION entry -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 3/5] partitions/msdos: enumerate also AIX LVM partitions
Graft AIX partitions enumeration in partitions/msdos.c
There is already a AIX disks detection logic in msdos.c. When an
AIX disk has been found, and if configured to, call the aix partitions
recognizer. This avoids removal of AIX disks protection from msdos.c,
avoids code duplication, and ensures that AIX partitions enumeration
is called before plain msdos partitions enumeration.
Signed-off-by: Philippe De Muyter
Cc: Karel Zak
Cc: Jens Axboe
Cc: Andrew Morton
---
block/partitions/msdos.c |5 +
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/block/partitions/msdos.c b/block/partitions/msdos.c
index 9bf19e6..9123f25 100644
--- a/block/partitions/msdos.c
+++ b/block/partitions/msdos.c
@@ -23,6 +23,7 @@
#include "check.h"
#include "msdos.h"
#include "efi.h"
+#include "aix.h"
/*
* Many architectures don't like unaligned accesses, while
@@ -462,8 +463,12 @@ int msdos_partition(struct parsed_partitions *state)
*/
if (aix_magic_present(state, data)) {
put_dev_sector(sect);
+#ifdef CONFIG_AIX_PARTITION
+ return aix_partition(state);
+#else
strlcat(state->pp_buf, " [AIX]", PAGE_SIZE);
return 0;
+#endif
}
if (!msdos_magic_present(data + 510)) {
--
1.7.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 5/5] partitions/Kconfig: add the AIX_PARTITION entry
This is the final patch enabling a user to select AIX lvm partitions detection. Signed-off-by: Philippe De Muyter Cc: Karel Zak Cc: Jens Axboe Cc: Andrew Morton --- block/partitions/Kconfig | 11 +++ 1 files changed, 11 insertions(+), 0 deletions(-) diff --git a/block/partitions/Kconfig b/block/partitions/Kconfig index 75a54e1..4cebb2f 100644 --- a/block/partitions/Kconfig +++ b/block/partitions/Kconfig @@ -68,6 +68,17 @@ config ACORN_PARTITION_RISCIX of machines called RISCiX. If you say 'Y' here, Linux will be able to read disks partitioned under RISCiX. +config AIX_PARTITION + bool "AIX basic partition table support" if PARTITION_ADVANCED + help + Say Y here if you would like to be able to read the hard disk + partition table format used by IBM or Motorola PowerPC machines + running AIX. AIX actually uses a Logical Volume Manager, where + "logical volumes" can be spread across one or multiple disks, + but this driver works only for the simple case of partitions which + are contiguous. + Otherwise, say N. + config OSF_PARTITION bool "Alpha OSF partition support" if PARTITION_ADVANCED default y if ALPHA -- 1.7.1 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 4/5] partitions/Makefile: compile aix.c if configured.
Signed-off-by: Philippe De Muyter Cc: Karel Zak Cc: Jens Axboe Cc: Andrew Morton --- block/partitions/Makefile |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/block/partitions/Makefile b/block/partitions/Makefile index 03af8ea..2be4d7b 100644 --- a/block/partitions/Makefile +++ b/block/partitions/Makefile @@ -7,6 +7,7 @@ obj-$(CONFIG_BLOCK) := check.o obj-$(CONFIG_ACORN_PARTITION) += acorn.o obj-$(CONFIG_AMIGA_PARTITION) += amiga.o obj-$(CONFIG_ATARI_PARTITION) += atari.o +obj-$(CONFIG_AIX_PARTITION) += aix.o obj-$(CONFIG_MAC_PARTITION) += mac.o obj-$(CONFIG_LDM_PARTITION) += ldm.o obj-$(CONFIG_MSDOS_PARTITION) += msdos.o -- 1.7.1 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/5] partitions/msdos.c: end-of-line whitespace and semicolon cleanup
Signed-off-by: Philippe De Muyter
Cc: Karel Zak
Cc: Jens Axboe
Cc: Andrew Morton
---
block/partitions/msdos.c | 12 ++--
1 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/block/partitions/msdos.c b/block/partitions/msdos.c
index 7681cd2..9bf19e6 100644
--- a/block/partitions/msdos.c
+++ b/block/partitions/msdos.c
@@ -90,7 +90,7 @@ static int aix_magic_present(struct parsed_partitions *state,
unsigned char *p)
if (d[0] == '_' && d[1] == 'L' && d[2] == 'V' && d[3] == 'M')
ret = 1;
put_dev_sector(sect);
- };
+ }
return ret;
}
@@ -142,7 +142,7 @@ static void parse_extended(struct parsed_partitions *state,
return;
if (!msdos_magic_present(data + 510))
- goto done;
+ goto done;
p = (struct partition *) (data + 0x1be);
@@ -155,7 +155,7 @@ static void parse_extended(struct parsed_partitions *state,
* and OS/2 seems to use all four entries.
*/
- /*
+ /*
* First process the data partition(s)
*/
for (i=0; i<4; i++, p++) {
@@ -263,7 +263,7 @@ static void parse_solaris_x86(struct parsed_partitions
*state,
}
#if defined(CONFIG_BSD_DISKLABEL)
-/*
+/*
* Create devices for BSD partitions listed in a disklabel, under a
* dos-like partition. See parse_extended() for more information.
*/
@@ -294,7 +294,7 @@ static void parse_bsd(struct parsed_partitions *state,
if (state->next == state->limit)
break;
- if (p->p_fstype == BSD_FS_UNUSED)
+ if (p->p_fstype == BSD_FS_UNUSED)
continue;
bsd_start = le32_to_cpu(p->p_offset);
bsd_size = le32_to_cpu(p->p_size);
@@ -441,7 +441,7 @@ static struct {
{NEW_SOLARIS_X86_PARTITION, parse_solaris_x86},
{0, NULL},
};
-
+
int msdos_partition(struct parsed_partitions *state)
{
sector_t sector_size = bdev_logical_block_size(state->bdev) / 512;
--
1.7.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] partitions/efi.c: replace useless kzalloc's by kmalloc's
In alloc_read_gpt_entries and alloc_read_gpt_header, the kzalloc'ated zones are either totally overwritten by the following read_lba call, or freed. As kmalloc is cheaper than kzalloc, use kmalloc. Signed-off-by: Philippe De Muyter Cc: Matt Domsch Cc: Panagiotis Issaris Cc: Jens Axboe Cc: Andrew Morton --- block/partitions/efi.c |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/block/partitions/efi.c b/block/partitions/efi.c index ff5804e..c85fc89 100644 --- a/block/partitions/efi.c +++ b/block/partitions/efi.c @@ -238,7 +238,7 @@ static gpt_entry *alloc_read_gpt_entries(struct parsed_partitions *state, le32_to_cpu(gpt->sizeof_partition_entry); if (!count) return NULL; - pte = kzalloc(count, GFP_KERNEL); + pte = kmalloc(count, GFP_KERNEL); if (!pte) return NULL; @@ -267,7 +267,7 @@ static gpt_header *alloc_read_gpt_header(struct parsed_partitions *state, gpt_header *gpt; unsigned ssz = bdev_logical_block_size(state->bdev); - gpt = kzalloc(ssz, GFP_KERNEL); + gpt = kmalloc(ssz, GFP_KERNEL); if (!gpt) return NULL; -- 1.7.1 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/5] Add aix lvm partitions support files
Hi Karel
On Mon, Apr 29, 2013 at 02:36:51PM +0200, Karel Zak wrote:
> On Mon, Apr 29, 2013 at 01:40:41PM +0200, Philippe De Muyter wrote:
> > > why not memset(pps_found, )? I also see magical constant 16
> >
> > Actually 16 is the maximum partition count allowed in a disk by linux,
> > or should it be 15 ? Is there already a constant for that ?
> > The AIX disk I tested with had only :) 11 partitions.
>
> I don't think it's correct to expect any hardcoded limit.
>
> The struct parsed_partitions->parts is allocated according to
> disk_max_parts() where the limit depends on number of minor numbers or
> it's DISK_MAX_PARTS (=256).
>
> There is no problem to create disk with many partitions:
>
> # modprobe scsi_debug dev_size_mb=300
> # (echo -e 'g\n'; for i in {1..100}; do echo -e "n\n\n\n+1M"; done; \
> echo -e 'w\nq\n') | fdisk /dev/sdb
>
> # lsblk -n /dev/sdb | wc -l
> 101
how are they named then ?
on my system (a 2.6.24 kernel which is the last one that support my
powerpc PReP machine because PReP support got removed with the merge
of /arch/ppc and /arch/powerpc :( ), I get :
root:~# ls -l /dev/sd[ab]*
brw-r- 1 root disk 8, 0 Apr 25 22:22 /dev/sda
brw-r- 1 root disk 8, 10 Apr 25 22:22 /dev/sda10
brw-r- 1 root disk 8, 11 Apr 25 22:22 /dev/sda11
brw-r- 1 root disk 8, 3 Apr 25 22:22 /dev/sda3
brw-r- 1 root disk 8, 4 Apr 25 22:22 /dev/sda4
brw-r- 1 root disk 8, 5 Apr 25 22:22 /dev/sda5
brw-r- 1 root disk 8, 6 Apr 25 22:22 /dev/sda6
brw-r- 1 root disk 8, 7 Apr 25 22:22 /dev/sda7
brw-r- 1 root disk 8, 8 Apr 25 22:22 /dev/sda8
brw-r- 1 root disk 8, 9 Apr 25 22:22 /dev/sda9
brw-r- 1 root disk 8, 16 Apr 25 22:22 /dev/sdb
brw-r- 1 root disk 8, 26 Apr 25 22:22 /dev/sdb10
brw-r- 1 root disk 8, 27 Apr 25 22:22 /dev/sdb11
brw-r- 1 root disk 8, 19 Apr 25 22:22 /dev/sdb3
brw-r- 1 root disk 8, 20 Apr 25 22:22 /dev/sdb4
brw-r- 1 root disk 8, 21 Apr 25 22:22 /dev/sdb5
brw-r- 1 root disk 8, 22 Apr 25 22:22 /dev/sdb6
brw-r- 1 root disk 8, 23 Apr 25 22:22 /dev/sdb7
brw-r- 1 root disk 8, 24 Apr 25 22:22 /dev/sdb8
brw-r- 1 root disk 8, 25 Apr 25 22:22 /dev/sdb9
root:~#
so sda is 8,0 and sdb is 8,16
and if, while discovering partitions of /dev/sda, I try to make a
partition 16 or higher, it is silently discarded by 'put_partition'.
Is that changed ?
Philippe
--
Philippe De Muyter +32 2 6101532 Macq SA rue de l'Aeronef 2 B-1140 Bruxelles
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/5] Add aix lvm partitions support files
On Mon, Apr 29, 2013 at 11:37:56AM +0200, Karel Zak wrote:
> On Thu, Apr 25, 2013 at 11:10:26PM +0200, Philippe De Muyter wrote:
Thanks for the interest and the quick reply.
> > +int aix_partition(struct parsed_partitions *state)
> > +{
> > + int ret = 0;
> > + Sector sect;
> > + unsigned char *d;
> > + u32 pp_bytes_size;
> > + u32 pp_blocks_size = 0;
> > + u32 vgda_sector = 0;
> > + u32 vgda_len = 0;
> > + int numlvs = 0;
> > + struct pvd *pvd;
> > + unsigned short pps_per_lv[16];
> > + unsigned short pps_found[16];
> > + unsigned char lv_is_contiguous[16];
> > + struct lvname *n = NULL;
> > +
> > + d = read_part_sector(state, 7, );
> > + if (d) {
> > + struct lvm_rec *p = (struct lvm_rec *)d;
> > + u16 lvm_version = be16_to_cpu(p->version);
> > + char tmp[64];
> > +
> > + if (lvm_version == 1) {
> > + int pp_size_log2 = be16_to_cpu(p->pp_size);
> > +
> > + pp_bytes_size = 1 << pp_size_log2;
> > + pp_blocks_size = pp_bytes_size / 512;
> > + snprintf(tmp, sizeof(tmp), " AIX LVM header version %u
> > found\n", lvm_version);
>
> 'tmp' is nowhere used, maybe you want to use strlcat(state->pp_buf, tmp,
> PAGE_SIZE); too.
Oops, of course :)
>
> > + vgda_len = be32_to_cpu(p->vgda_len);
> > + vgda_sector = be32_to_cpu(p->vgda_psn[0]);
> > + } else {
> > + snprintf(tmp, sizeof(tmp), " unsupported AIX LVM
> > version %d found\n",
> > + lvm_version);
> > + strlcat(state->pp_buf, tmp, PAGE_SIZE);
> > + }
> > + put_dev_sector(sect);
> > + }
> > + if (vgda_sector && (d = read_part_sector(state, vgda_sector, ))) {
> > + struct vgda *p = (struct vgda *)d;
> > +
> > + numlvs = be16_to_cpu(p->numlvs);
> > + put_dev_sector(sect);
> > + }
> > + if (numlvs && (d = read_part_sector(state, vgda_sector + 1, ))) {
> > + struct lvd *p = (struct lvd *)d;
> > + int i;
> > +
> > + n = alloc_lvn(state, vgda_sector + vgda_len - 33);
> > + if (n) {
> > + int j = 0;
> > +
> > + memset(lv_is_contiguous, 0, 16);
> > + for (i = 0; i < 16; i += 1)
> > + pps_found[i] = 0;
>
> why not memset(pps_found, )? I also see magical constant 16
Actually 16 is the maximum partition count allowed in a disk by linux,
or should it be 15 ? Is there already a constant for that ?
The AIX disk I tested with had only :) 11 partitions.
> everywhere, maybe you can use sizeof() and ARRAY_SIZE().
Will do.
>
> > + for (i = 0; j < numlvs && i < 16; i += 1) {
> > + pps_per_lv[i] = be16_to_cpu(p[i].num_lps);
> > + if (pps_per_lv[i])
> > + j += 1;
> > + }
>
> hmm, what's wrong with j++ and i++, "j += 1" seems like old Python :-)
>
What's wrong with "j += 1" ?
I only use ++ for side effects; I personaly find "j += 1" more readable :)
But I should rename 'j' to be more explicit.
> > + while (i < 16)
> > + pps_per_lv[i++] = 0;
> > + }
> > + put_dev_sector(sect);
> > + }
> > + pvd = alloc_pvd(state, vgda_sector + 17);
> > + if (pvd) {
> > + int numpps = be16_to_cpu(pvd->pp_count);
> > + int psn_part1 = be32_to_cpu(pvd->psn_part1);
> > + int i;
> > + int cur_lv_ix = -1;
> > + int next_lp_ix = 1;
> > + int lp_ix;
> > +
> > + for (i = 0; i < numpps; i += 1) {
> > + struct ppe *p = pvd->ppe + i;
> > + int lv_ix;
> > +
> > + lp_ix = be16_to_cpu(p->lp_ix);
> > + if (!lp_ix) {
> > + next_lp_ix = 1;
> > + continue;
> > + }
> > + lv_ix = be16_to_cpu(p->lv_ix) - 1;
> > + pps_found[lv_ix] += 1;
>
> Would be better to be a little paranoid when you read the data from
> disk and check that lv_ix is in range 0..16 ?
Of course.
>
Re: [PATCH 2/5] Add aix lvm partitions support files
On Mon, Apr 29, 2013 at 11:37:56AM +0200, Karel Zak wrote:
On Thu, Apr 25, 2013 at 11:10:26PM +0200, Philippe De Muyter wrote:
Thanks for the interest and the quick reply.
+int aix_partition(struct parsed_partitions *state)
+{
+ int ret = 0;
+ Sector sect;
+ unsigned char *d;
+ u32 pp_bytes_size;
+ u32 pp_blocks_size = 0;
+ u32 vgda_sector = 0;
+ u32 vgda_len = 0;
+ int numlvs = 0;
+ struct pvd *pvd;
+ unsigned short pps_per_lv[16];
+ unsigned short pps_found[16];
+ unsigned char lv_is_contiguous[16];
+ struct lvname *n = NULL;
+
+ d = read_part_sector(state, 7, sect);
+ if (d) {
+ struct lvm_rec *p = (struct lvm_rec *)d;
+ u16 lvm_version = be16_to_cpu(p-version);
+ char tmp[64];
+
+ if (lvm_version == 1) {
+ int pp_size_log2 = be16_to_cpu(p-pp_size);
+
+ pp_bytes_size = 1 pp_size_log2;
+ pp_blocks_size = pp_bytes_size / 512;
+ snprintf(tmp, sizeof(tmp), AIX LVM header version %u
found\n, lvm_version);
'tmp' is nowhere used, maybe you want to use strlcat(state-pp_buf, tmp,
PAGE_SIZE); too.
Oops, of course :)
+ vgda_len = be32_to_cpu(p-vgda_len);
+ vgda_sector = be32_to_cpu(p-vgda_psn[0]);
+ } else {
+ snprintf(tmp, sizeof(tmp), unsupported AIX LVM
version %d found\n,
+ lvm_version);
+ strlcat(state-pp_buf, tmp, PAGE_SIZE);
+ }
+ put_dev_sector(sect);
+ }
+ if (vgda_sector (d = read_part_sector(state, vgda_sector, sect))) {
+ struct vgda *p = (struct vgda *)d;
+
+ numlvs = be16_to_cpu(p-numlvs);
+ put_dev_sector(sect);
+ }
+ if (numlvs (d = read_part_sector(state, vgda_sector + 1, sect))) {
+ struct lvd *p = (struct lvd *)d;
+ int i;
+
+ n = alloc_lvn(state, vgda_sector + vgda_len - 33);
+ if (n) {
+ int j = 0;
+
+ memset(lv_is_contiguous, 0, 16);
+ for (i = 0; i 16; i += 1)
+ pps_found[i] = 0;
why not memset(pps_found, )? I also see magical constant 16
Actually 16 is the maximum partition count allowed in a disk by linux,
or should it be 15 ? Is there already a constant for that ?
The AIX disk I tested with had only :) 11 partitions.
everywhere, maybe you can use sizeof() and ARRAY_SIZE().
Will do.
+ for (i = 0; j numlvs i 16; i += 1) {
+ pps_per_lv[i] = be16_to_cpu(p[i].num_lps);
+ if (pps_per_lv[i])
+ j += 1;
+ }
hmm, what's wrong with j++ and i++, j += 1 seems like old Python :-)
What's wrong with j += 1 ?
I only use ++ for side effects; I personaly find j += 1 more readable :)
But I should rename 'j' to be more explicit.
+ while (i 16)
+ pps_per_lv[i++] = 0;
+ }
+ put_dev_sector(sect);
+ }
+ pvd = alloc_pvd(state, vgda_sector + 17);
+ if (pvd) {
+ int numpps = be16_to_cpu(pvd-pp_count);
+ int psn_part1 = be32_to_cpu(pvd-psn_part1);
+ int i;
+ int cur_lv_ix = -1;
+ int next_lp_ix = 1;
+ int lp_ix;
+
+ for (i = 0; i numpps; i += 1) {
+ struct ppe *p = pvd-ppe + i;
+ int lv_ix;
+
+ lp_ix = be16_to_cpu(p-lp_ix);
+ if (!lp_ix) {
+ next_lp_ix = 1;
+ continue;
+ }
+ lv_ix = be16_to_cpu(p-lv_ix) - 1;
+ pps_found[lv_ix] += 1;
Would be better to be a little paranoid when you read the data from
disk and check that lv_ix is in range 0..16 ?
Of course.
+ if (lp_ix != next_lp_ix)
+ continue;
+ if (lp_ix == 1)
+ cur_lv_ix = lv_ix;
+ else if (lv_ix != cur_lv_ix)
+ next_lp_ix = 1;
+ if (lp_ix == pps_per_lv[lv_ix]) {
+ char tmp[70];
+
+ put_partition(state, lv_ix + 1,
+ (i + 1 - lp_ix) * pp_blocks_size +
psn_part1,
+ pps_per_lv[lv_ix] * pp_blocks_size);
+ snprintf(tmp, sizeof(tmp), %s\n,
n[lv_ix].name);
+ strlcat(state-pp_buf, tmp, PAGE_SIZE);
+ lv_is_contiguous[lv_ix] = 1;
+ ret = 1;
+ next_lp_ix = 1
Re: [PATCH 2/5] Add aix lvm partitions support files
Hi Karel
On Mon, Apr 29, 2013 at 02:36:51PM +0200, Karel Zak wrote:
On Mon, Apr 29, 2013 at 01:40:41PM +0200, Philippe De Muyter wrote:
why not memset(pps_found, )? I also see magical constant 16
Actually 16 is the maximum partition count allowed in a disk by linux,
or should it be 15 ? Is there already a constant for that ?
The AIX disk I tested with had only :) 11 partitions.
I don't think it's correct to expect any hardcoded limit.
The struct parsed_partitions-parts is allocated according to
disk_max_parts() where the limit depends on number of minor numbers or
it's DISK_MAX_PARTS (=256).
There is no problem to create disk with many partitions:
# modprobe scsi_debug dev_size_mb=300
# (echo -e 'g\n'; for i in {1..100}; do echo -e n\n\n\n+1M; done; \
echo -e 'w\nq\n') | fdisk /dev/sdb
# lsblk -n /dev/sdb | wc -l
101
how are they named then ?
on my system (a 2.6.24 kernel which is the last one that support my
powerpc PReP machine because PReP support got removed with the merge
of /arch/ppc and /arch/powerpc :( ), I get :
root:~# ls -l /dev/sd[ab]*
brw-r- 1 root disk 8, 0 Apr 25 22:22 /dev/sda
brw-r- 1 root disk 8, 10 Apr 25 22:22 /dev/sda10
brw-r- 1 root disk 8, 11 Apr 25 22:22 /dev/sda11
brw-r- 1 root disk 8, 3 Apr 25 22:22 /dev/sda3
brw-r- 1 root disk 8, 4 Apr 25 22:22 /dev/sda4
brw-r- 1 root disk 8, 5 Apr 25 22:22 /dev/sda5
brw-r- 1 root disk 8, 6 Apr 25 22:22 /dev/sda6
brw-r- 1 root disk 8, 7 Apr 25 22:22 /dev/sda7
brw-r- 1 root disk 8, 8 Apr 25 22:22 /dev/sda8
brw-r- 1 root disk 8, 9 Apr 25 22:22 /dev/sda9
brw-r- 1 root disk 8, 16 Apr 25 22:22 /dev/sdb
brw-r- 1 root disk 8, 26 Apr 25 22:22 /dev/sdb10
brw-r- 1 root disk 8, 27 Apr 25 22:22 /dev/sdb11
brw-r- 1 root disk 8, 19 Apr 25 22:22 /dev/sdb3
brw-r- 1 root disk 8, 20 Apr 25 22:22 /dev/sdb4
brw-r- 1 root disk 8, 21 Apr 25 22:22 /dev/sdb5
brw-r- 1 root disk 8, 22 Apr 25 22:22 /dev/sdb6
brw-r- 1 root disk 8, 23 Apr 25 22:22 /dev/sdb7
brw-r- 1 root disk 8, 24 Apr 25 22:22 /dev/sdb8
brw-r- 1 root disk 8, 25 Apr 25 22:22 /dev/sdb9
root:~#
so sda is 8,0 and sdb is 8,16
and if, while discovering partitions of /dev/sda, I try to make a
partition 16 or higher, it is silently discarded by 'put_partition'.
Is that changed ?
Philippe
--
Philippe De Muyter +32 2 6101532 Macq SA rue de l'Aeronef 2 B-1140 Bruxelles
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] partitions/efi.c: replace useless kzalloc's by kmalloc's
In alloc_read_gpt_entries and alloc_read_gpt_header, the kzalloc'ated zones are either totally overwritten by the following read_lba call, or freed. As kmalloc is cheaper than kzalloc, use kmalloc. Signed-off-by: Philippe De Muyter p...@macqel.be Cc: Matt Domsch matt_dom...@dell.com Cc: Panagiotis Issaris ta...@issaris.org Cc: Jens Axboe ax...@kernel.dk Cc: Andrew Morton a...@linux-foundation.org --- block/partitions/efi.c |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/block/partitions/efi.c b/block/partitions/efi.c index ff5804e..c85fc89 100644 --- a/block/partitions/efi.c +++ b/block/partitions/efi.c @@ -238,7 +238,7 @@ static gpt_entry *alloc_read_gpt_entries(struct parsed_partitions *state, le32_to_cpu(gpt-sizeof_partition_entry); if (!count) return NULL; - pte = kzalloc(count, GFP_KERNEL); + pte = kmalloc(count, GFP_KERNEL); if (!pte) return NULL; @@ -267,7 +267,7 @@ static gpt_header *alloc_read_gpt_header(struct parsed_partitions *state, gpt_header *gpt; unsigned ssz = bdev_logical_block_size(state-bdev); - gpt = kzalloc(ssz, GFP_KERNEL); + gpt = kmalloc(ssz, GFP_KERNEL); if (!gpt) return NULL; -- 1.7.1 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/5] partitions/msdos.c: end-of-line whitespace and semicolon cleanup
Signed-off-by: Philippe De Muyter p...@macqel.be
Cc: Karel Zak k...@redhat.com
Cc: Jens Axboe ax...@kernel.dk
Cc: Andrew Morton a...@linux-foundation.org
---
block/partitions/msdos.c | 12 ++--
1 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/block/partitions/msdos.c b/block/partitions/msdos.c
index 7681cd2..9bf19e6 100644
--- a/block/partitions/msdos.c
+++ b/block/partitions/msdos.c
@@ -90,7 +90,7 @@ static int aix_magic_present(struct parsed_partitions *state,
unsigned char *p)
if (d[0] == '_' d[1] == 'L' d[2] == 'V' d[3] == 'M')
ret = 1;
put_dev_sector(sect);
- };
+ }
return ret;
}
@@ -142,7 +142,7 @@ static void parse_extended(struct parsed_partitions *state,
return;
if (!msdos_magic_present(data + 510))
- goto done;
+ goto done;
p = (struct partition *) (data + 0x1be);
@@ -155,7 +155,7 @@ static void parse_extended(struct parsed_partitions *state,
* and OS/2 seems to use all four entries.
*/
- /*
+ /*
* First process the data partition(s)
*/
for (i=0; i4; i++, p++) {
@@ -263,7 +263,7 @@ static void parse_solaris_x86(struct parsed_partitions
*state,
}
#if defined(CONFIG_BSD_DISKLABEL)
-/*
+/*
* Create devices for BSD partitions listed in a disklabel, under a
* dos-like partition. See parse_extended() for more information.
*/
@@ -294,7 +294,7 @@ static void parse_bsd(struct parsed_partitions *state,
if (state-next == state-limit)
break;
- if (p-p_fstype == BSD_FS_UNUSED)
+ if (p-p_fstype == BSD_FS_UNUSED)
continue;
bsd_start = le32_to_cpu(p-p_offset);
bsd_size = le32_to_cpu(p-p_size);
@@ -441,7 +441,7 @@ static struct {
{NEW_SOLARIS_X86_PARTITION, parse_solaris_x86},
{0, NULL},
};
-
+
int msdos_partition(struct parsed_partitions *state)
{
sector_t sector_size = bdev_logical_block_size(state-bdev) / 512;
--
1.7.1
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
