Re: [PATCH 06/44 take 2] [UBI] startup code
Christoph, On Thu, 2007-05-17 at 15:44 +0100, Christoph Hellwig wrote: > So both I and Rusty told you not to do this and you did it anyway. > > I'm quite pissed about this ignorance. Andrew, what do we do about such > a case? Should we just revert ubi until they fixed their mess up? Just for reference: http://lkml.org/lkml/2007/2/19/67 Sorry, I thought Rusty meant that we can use it if we invent our own type, see the first part of his answer. You did not answer Frank's mail about _why_ we use it. So I thought you agreed with us, I did not ignore you, no need to be so assertive. It is not a problem to fix this, but neither Rusty nor you explained what is wrong with that call. But we _did_ explain why we use it. You ignored the explanation and the questions. -- Best regards, Artem Bityutskiy (Битюцкий Артём) - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 06/44 take 2] [UBI] startup code
On Sun, Feb 25, 2007 at 05:58:28AM +, Christoph Hellwig wrote: > > Why not? We tried to avoid this but found out that this is the most > > decent interface. Specific advises are welcome. > > because this type of compount interface is really painful for the user. > the module.param=foo syntax makes sure paramaters can be used without > endless documentation for each and every single of them, and makes > sure module writers don't introduce bugs in their own parser > reimplementations. > > Rusty, was it intentional that drivers can use __module_param_call? > Do you think the ubi use here is okay? So both I and Rusty told you not to do this and you did it anyway. I'm quite pissed about this ignorance. Andrew, what do we do about such a case? Should we just revert ubi until they fixed their mess up? - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 06/44 take 2] [UBI] startup code
On Sun, Feb 25, 2007 at 05:58:28AM +, Christoph Hellwig wrote: Why not? We tried to avoid this but found out that this is the most decent interface. Specific advises are welcome. because this type of compount interface is really painful for the user. the module.param=foo syntax makes sure paramaters can be used without endless documentation for each and every single of them, and makes sure module writers don't introduce bugs in their own parser reimplementations. Rusty, was it intentional that drivers can use __module_param_call? Do you think the ubi use here is okay? So both I and Rusty told you not to do this and you did it anyway. I'm quite pissed about this ignorance. Andrew, what do we do about such a case? Should we just revert ubi until they fixed their mess up? - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 06/44 take 2] [UBI] startup code
Christoph, On Thu, 2007-05-17 at 15:44 +0100, Christoph Hellwig wrote: So both I and Rusty told you not to do this and you did it anyway. I'm quite pissed about this ignorance. Andrew, what do we do about such a case? Should we just revert ubi until they fixed their mess up? Just for reference: http://lkml.org/lkml/2007/2/19/67 Sorry, I thought Rusty meant that we can use it if we invent our own type, see the first part of his answer. You did not answer Frank's mail about _why_ we use it. So I thought you agreed with us, I did not ignore you, no need to be so assertive. It is not a problem to fix this, but neither Rusty nor you explained what is wrong with that call. But we _did_ explain why we use it. You ignored the explanation and the questions. -- Best regards, Artem Bityutskiy (Битюцкий Артём) - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 06/44 take 2] [UBI] startup code
Hi Rusty, On Mon, 2007-02-26 at 09:03 +1100, Rusty Russell wrote: > On Sun, 2007-02-25 at 05:58 +, Christoph Hellwig wrote: > > On Tue, Feb 20, 2007 at 03:00:56PM +0200, Artem Bityutskiy wrote: > > > > > +module_param_call(mtd, ubi_mtd_param_parse, NULL, NULL, 000); > > > > > +MODULE_PARM_DESC(mtd, "MTD devices to attach. Parameter format: " > > > > > + "mtd=[,,]. " > > > > > + "Multiple \"mtd\" parameters may be specified.\n" > > > > > + "MTD devices may be specified by their number or > > > > > name. " > > > > > + "Optional \"vid_hdr_offs\" and \"data_offs\" > > > > > parameters " > > > > > + "specify UBI VID header position and data > > > > > starting " > > > > > + "position to be used by UBI.\n" > > > > > + "Example: mtd=content,1984,2048 mtd=4 - attach > > > > > MTD device" > > > > > + "with name content using VID header offset 1984 > > > > > and data " > > > > > + "start 2048, and MTD device number 4 using > > > > > default " > > > > > + "offsets"); > > > > > The reason drivers can use __module_param_call is that they can > implement their own "types" for module parameters, which will end up > requiring this. > > Using it directly is only really for backwards compatibility (which is > important!), but for new parameters, this multi-part self-parsing is > nasty. Standard (but admittedly suboptimal) way to do this is having > three parameters module_param_array(name, ...), > module_param_array(header_offset, ...), > module_param_array(data_start, ...). we wanted to be able to ommit some of the parameters and let UBI use resonable defaults if needed. Example where we wanted to explicitly overwrite the offset parameters: mtd=content,1984,2048 Example where we did not want it but let instead UBI figure out resonable parameters: mtd=4 With the three arrays we did not see a nice way of achieving this. ubimtds=content,4,7 hdroffs=1984,?,1984 dataoff=s2048,?,2048 The ? look odd, don't they? We also looked at the slram parameters which are like this: slram=name0,addr0,+size0,name1,addr1,+size1 and found that if we wanted to add one more parameter e.g. width it cause likely an interface change: slram=name0,addr0,+size0,width0,name1,addr1,+size1,width1 Finally we found that we liked the phram parameter parsing (which is essentially the same) and did it similar to that. Frank - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 06/44 take 2] [UBI] startup code
Hi Rusty, On Mon, 2007-02-26 at 09:03 +1100, Rusty Russell wrote: On Sun, 2007-02-25 at 05:58 +, Christoph Hellwig wrote: On Tue, Feb 20, 2007 at 03:00:56PM +0200, Artem Bityutskiy wrote: +module_param_call(mtd, ubi_mtd_param_parse, NULL, NULL, 000); +MODULE_PARM_DESC(mtd, MTD devices to attach. Parameter format: + mtd=name|num[,vid_hdr_offs,data_offs]. + Multiple \mtd\ parameters may be specified.\n + MTD devices may be specified by their number or name. + Optional \vid_hdr_offs\ and \data_offs\ parameters + specify UBI VID header position and data starting + position to be used by UBI.\n + Example: mtd=content,1984,2048 mtd=4 - attach MTD device + with name content using VID header offset 1984 and data + start 2048, and MTD device number 4 using default + offsets); The reason drivers can use __module_param_call is that they can implement their own types for module parameters, which will end up requiring this. Using it directly is only really for backwards compatibility (which is important!), but for new parameters, this multi-part self-parsing is nasty. Standard (but admittedly suboptimal) way to do this is having three parameters module_param_array(name, ...), module_param_array(header_offset, ...), module_param_array(data_start, ...). we wanted to be able to ommit some of the parameters and let UBI use resonable defaults if needed. Example where we wanted to explicitly overwrite the offset parameters: mtd=content,1984,2048 Example where we did not want it but let instead UBI figure out resonable parameters: mtd=4 With the three arrays we did not see a nice way of achieving this. ubimtds=content,4,7 hdroffs=1984,?,1984 dataoff=s2048,?,2048 The ? look odd, don't they? We also looked at the slram parameters which are like this: slram=name0,addr0,+size0,name1,addr1,+size1 and found that if we wanted to add one more parameter e.g. width it cause likely an interface change: slram=name0,addr0,+size0,width0,name1,addr1,+size1,width1 Finally we found that we liked the phram parameter parsing (which is essentially the same) and did it similar to that. Frank - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 06/44 take 2] [UBI] startup code
On Sun, 2007-02-25 at 05:58 +, Christoph Hellwig wrote: > because this type of compount interface is really painful for the user. > the module.param=foo syntax makes sure paramaters can be used without > endless documentation for each and every single of them, and makes > sure module writers don't introduce bugs in their own parser > reimplementations. I stand that without the use this function the interface is much worse. I will be grateful if you advice something better. -- Best regards, Artem Bityutskiy (Битюцкий Артём) - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 06/44 take 2] [UBI] startup code
On Sun, 2007-02-25 at 05:58 +, Christoph Hellwig wrote: because this type of compount interface is really painful for the user. the module.param=foo syntax makes sure paramaters can be used without endless documentation for each and every single of them, and makes sure module writers don't introduce bugs in their own parser reimplementations. I stand that without the use this function the interface is much worse. I will be grateful if you advice something better. -- Best regards, Artem Bityutskiy (Битюцкий Артём) - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 06/44 take 2] [UBI] startup code
On Sun, 2007-02-25 at 05:58 +, Christoph Hellwig wrote: > On Tue, Feb 20, 2007 at 03:00:56PM +0200, Artem Bityutskiy wrote: > > > > +module_param_call(mtd, ubi_mtd_param_parse, NULL, NULL, 000); > > > > +MODULE_PARM_DESC(mtd, "MTD devices to attach. Parameter format: " > > > > + "mtd=[,,]. " > > > > + "Multiple \"mtd\" parameters may be specified.\n" > > > > + "MTD devices may be specified by their number or > > > > name. " > > > > + "Optional \"vid_hdr_offs\" and \"data_offs\" > > > > parameters " > > > > + "specify UBI VID header position and data > > > > starting " > > > > + "position to be used by UBI.\n" > > > > + "Example: mtd=content,1984,2048 mtd=4 - attach > > > > MTD device" > > > > + "with name content using VID header offset 1984 > > > > and data " > > > > + "start 2048, and MTD device number 4 using > > > > default " > > > > + "offsets"); > > > > > > This is a very odd paramater interface. We really don't want drivers to > > > use > > > module_param_call directly. You probably want various module_param_array > > > calls > > > instead. > > > > Why not? We tried to avoid this but found out that this is the most > > decent interface. Specific advises are welcome. > > because this type of compount interface is really painful for the user. > the module.param=foo syntax makes sure paramaters can be used without > endless documentation for each and every single of them, and makes > sure module writers don't introduce bugs in their own parser > reimplementations. > > Rusty, was it intentional that drivers can use __module_param_call? > Do you think the ubi use here is okay? The reason drivers can use __module_param_call is that they can implement their own "types" for module parameters, which will end up requiring this. Using it directly is only really for backwards compatibility (which is important!), but for new parameters, this multi-part self-parsing is nasty. Standard (but admittedly suboptimal) way to do this is having three parameters module_param_array(name, ...), module_param_array(header_offset, ...), module_param_array(data_start, ...). Hope that helps, Rusty. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 06/44 take 2] [UBI] startup code
On Sun, 2007-02-25 at 05:58 +, Christoph Hellwig wrote: On Tue, Feb 20, 2007 at 03:00:56PM +0200, Artem Bityutskiy wrote: +module_param_call(mtd, ubi_mtd_param_parse, NULL, NULL, 000); +MODULE_PARM_DESC(mtd, MTD devices to attach. Parameter format: + mtd=name|num[,vid_hdr_offs,data_offs]. + Multiple \mtd\ parameters may be specified.\n + MTD devices may be specified by their number or name. + Optional \vid_hdr_offs\ and \data_offs\ parameters + specify UBI VID header position and data starting + position to be used by UBI.\n + Example: mtd=content,1984,2048 mtd=4 - attach MTD device + with name content using VID header offset 1984 and data + start 2048, and MTD device number 4 using default + offsets); This is a very odd paramater interface. We really don't want drivers to use module_param_call directly. You probably want various module_param_array calls instead. Why not? We tried to avoid this but found out that this is the most decent interface. Specific advises are welcome. because this type of compount interface is really painful for the user. the module.param=foo syntax makes sure paramaters can be used without endless documentation for each and every single of them, and makes sure module writers don't introduce bugs in their own parser reimplementations. Rusty, was it intentional that drivers can use __module_param_call? Do you think the ubi use here is okay? The reason drivers can use __module_param_call is that they can implement their own types for module parameters, which will end up requiring this. Using it directly is only really for backwards compatibility (which is important!), but for new parameters, this multi-part self-parsing is nasty. Standard (but admittedly suboptimal) way to do this is having three parameters module_param_array(name, ...), module_param_array(header_offset, ...), module_param_array(data_start, ...). Hope that helps, Rusty. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 06/44 take 2] [UBI] startup code
On Tue, Feb 20, 2007 at 03:00:56PM +0200, Artem Bityutskiy wrote: > > > +module_param_call(mtd, ubi_mtd_param_parse, NULL, NULL, 000); > > > +MODULE_PARM_DESC(mtd, "MTD devices to attach. Parameter format: " > > > + "mtd=[,,]. " > > > + "Multiple \"mtd\" parameters may be specified.\n" > > > + "MTD devices may be specified by their number or name. " > > > + "Optional \"vid_hdr_offs\" and \"data_offs\" parameters " > > > + "specify UBI VID header position and data starting " > > > + "position to be used by UBI.\n" > > > + "Example: mtd=content,1984,2048 mtd=4 - attach MTD device" > > > + "with name content using VID header offset 1984 and data " > > > + "start 2048, and MTD device number 4 using default " > > > + "offsets"); > > > > This is a very odd paramater interface. We really don't want drivers to use > > module_param_call directly. You probably want various module_param_array > > calls > > instead. > > Why not? We tried to avoid this but found out that this is the most > decent interface. Specific advises are welcome. because this type of compount interface is really painful for the user. the module.param=foo syntax makes sure paramaters can be used without endless documentation for each and every single of them, and makes sure module writers don't introduce bugs in their own parser reimplementations. Rusty, was it intentional that drivers can use __module_param_call? Do you think the ubi use here is okay? - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 06/44 take 2] [UBI] startup code
On Tue, Feb 20, 2007 at 03:00:56PM +0200, Artem Bityutskiy wrote: +module_param_call(mtd, ubi_mtd_param_parse, NULL, NULL, 000); +MODULE_PARM_DESC(mtd, MTD devices to attach. Parameter format: + mtd=name|num[,vid_hdr_offs,data_offs]. + Multiple \mtd\ parameters may be specified.\n + MTD devices may be specified by their number or name. + Optional \vid_hdr_offs\ and \data_offs\ parameters + specify UBI VID header position and data starting + position to be used by UBI.\n + Example: mtd=content,1984,2048 mtd=4 - attach MTD device + with name content using VID header offset 1984 and data + start 2048, and MTD device number 4 using default + offsets); This is a very odd paramater interface. We really don't want drivers to use module_param_call directly. You probably want various module_param_array calls instead. Why not? We tried to avoid this but found out that this is the most decent interface. Specific advises are welcome. because this type of compount interface is really painful for the user. the module.param=foo syntax makes sure paramaters can be used without endless documentation for each and every single of them, and makes sure module writers don't introduce bugs in their own parser reimplementations. Rusty, was it intentional that drivers can use __module_param_call? Do you think the ubi use here is okay? - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 06/44 take 2] [UBI] startup code
On Tue, 2007-02-20 at 15:00 +0200, Artem Bityutskiy wrote: > > This looks very odd. > > What exactly? I see, will be fixed, thanks. -- Best regards, Artem Bityutskiy (Битюцкий Артём) - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 06/44 take 2] [UBI] startup code
On Tue, 2007-02-20 at 15:00 +0200, Artem Bityutskiy wrote: This looks very odd. What exactly? I see, will be fixed, thanks. -- Best regards, Artem Bityutskiy (Битюцкий Артём) - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 06/44 take 2] [UBI] startup code
On Mon, 2007-02-19 at 10:59 +, Christoph Hellwig wrote:
> On Sat, Feb 17, 2007 at 06:54:54PM +0200, Artem Bityutskiy wrote:
> > +/* UBI headers must take 64 bytes. The below is a hacky way to ensure this
> > */
> > +static int __ubi_check_ec_hdr_size[(UBI_EC_HDR_SIZE == 64) - 1]
> > +__attribute__ ((__unused__));
> > +static int __ubi_check_ec_hdr_size[(UBI_VID_HDR_SIZE == 64) - 1]
> > +__attribute__ ((__unused__));
>
> please use BUILD_BUG_ON instead.
Will be done, thanks.
> > +
> > +static int ubi_attach_mtd_dev(const char *mtd_dev, int vid_hdr_offset,
> > + int data_offset);
> > +static void ubi_destroy_dev(int ubi_num);
>
> Can you reorder the code to avoid all these forward declarations please?
Could you please submit a CodingStyle patch that would contain a
requirement to use the "higher-level functions at the bottom,
lower-layer at top"? Because I just use the opposite.
> > + /* Attach MTD devices */
> > + for (i = 0; i < mtd_devs; i++) {
> > + struct mtd_dev_param *p = _dev_param[i];
> > +
> > + cond_resched();
> > + err = -EINVAL;
>
> This looks very odd.
What exactly?
> > +module_param_call(mtd, ubi_mtd_param_parse, NULL, NULL, 000);
> > +MODULE_PARM_DESC(mtd, "MTD devices to attach. Parameter format: "
> > + "mtd=[,,]. "
> > + "Multiple \"mtd\" parameters may be specified.\n"
> > + "MTD devices may be specified by their number or name. "
> > + "Optional \"vid_hdr_offs\" and \"data_offs\" parameters "
> > + "specify UBI VID header position and data starting "
> > + "position to be used by UBI.\n"
> > + "Example: mtd=content,1984,2048 mtd=4 - attach MTD device"
> > + "with name content using VID header offset 1984 and data "
> > + "start 2048, and MTD device number 4 using default "
> > + "offsets");
>
> This is a very odd paramater interface. We really don't want drivers to use
> module_param_call directly. You probably want various module_param_array
> calls
> instead.
Why not? We tried to avoid this but found out that this is the most
decent interface. Specific advises are welcome.
Thank you,
Artem.
--
Best regards,
Artem Bityutskiy (Битюцкий Артём)
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 06/44 take 2] [UBI] startup code
On Mon, 2007-02-19 at 10:59 +, Christoph Hellwig wrote:
On Sat, Feb 17, 2007 at 06:54:54PM +0200, Artem Bityutskiy wrote:
+/* UBI headers must take 64 bytes. The below is a hacky way to ensure this
*/
+static int __ubi_check_ec_hdr_size[(UBI_EC_HDR_SIZE == 64) - 1]
+__attribute__ ((__unused__));
+static int __ubi_check_ec_hdr_size[(UBI_VID_HDR_SIZE == 64) - 1]
+__attribute__ ((__unused__));
please use BUILD_BUG_ON instead.
Will be done, thanks.
+
+static int ubi_attach_mtd_dev(const char *mtd_dev, int vid_hdr_offset,
+ int data_offset);
+static void ubi_destroy_dev(int ubi_num);
Can you reorder the code to avoid all these forward declarations please?
Could you please submit a CodingStyle patch that would contain a
requirement to use the higher-level functions at the bottom,
lower-layer at top? Because I just use the opposite.
+ /* Attach MTD devices */
+ for (i = 0; i mtd_devs; i++) {
+ struct mtd_dev_param *p = mtd_dev_param[i];
+
+ cond_resched();
+ err = -EINVAL;
This looks very odd.
What exactly?
+module_param_call(mtd, ubi_mtd_param_parse, NULL, NULL, 000);
+MODULE_PARM_DESC(mtd, MTD devices to attach. Parameter format:
+ mtd=name|num[,vid_hdr_offs,data_offs].
+ Multiple \mtd\ parameters may be specified.\n
+ MTD devices may be specified by their number or name.
+ Optional \vid_hdr_offs\ and \data_offs\ parameters
+ specify UBI VID header position and data starting
+ position to be used by UBI.\n
+ Example: mtd=content,1984,2048 mtd=4 - attach MTD device
+ with name content using VID header offset 1984 and data
+ start 2048, and MTD device number 4 using default
+ offsets);
This is a very odd paramater interface. We really don't want drivers to use
module_param_call directly. You probably want various module_param_array
calls
instead.
Why not? We tried to avoid this but found out that this is the most
decent interface. Specific advises are welcome.
Thank you,
Artem.
--
Best regards,
Artem Bityutskiy (Битюцкий Артём)
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 06/44 take 2] [UBI] startup code
On Sat, Feb 17, 2007 at 06:54:54PM +0200, Artem Bityutskiy wrote:
> +/* UBI headers must take 64 bytes. The below is a hacky way to ensure this */
> +static int __ubi_check_ec_hdr_size[(UBI_EC_HDR_SIZE == 64) - 1]
> +__attribute__ ((__unused__));
> +static int __ubi_check_ec_hdr_size[(UBI_VID_HDR_SIZE == 64) - 1]
> +__attribute__ ((__unused__));
please use BUILD_BUG_ON instead.
> +
> +static int ubi_attach_mtd_dev(const char *mtd_dev, int vid_hdr_offset,
> + int data_offset);
> +static void ubi_destroy_dev(int ubi_num);
Can you reorder the code to avoid all these forward declarations please?
> + /* Attach MTD devices */
> + for (i = 0; i < mtd_devs; i++) {
> + struct mtd_dev_param *p = _dev_param[i];
> +
> + cond_resched();
> + err = -EINVAL;
This looks very odd.
> +module_param_call(mtd, ubi_mtd_param_parse, NULL, NULL, 000);
> +MODULE_PARM_DESC(mtd, "MTD devices to attach. Parameter format: "
> + "mtd=[,,]. "
> + "Multiple \"mtd\" parameters may be specified.\n"
> + "MTD devices may be specified by their number or name. "
> + "Optional \"vid_hdr_offs\" and \"data_offs\" parameters "
> + "specify UBI VID header position and data starting "
> + "position to be used by UBI.\n"
> + "Example: mtd=content,1984,2048 mtd=4 - attach MTD device"
> + "with name content using VID header offset 1984 and data "
> + "start 2048, and MTD device number 4 using default "
> + "offsets");
This is a very odd paramater interface. We really don't want drivers to use
module_param_call directly. You probably want various module_param_array calls
instead.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 06/44 take 2] [UBI] startup code
On Sat, Feb 17, 2007 at 06:54:54PM +0200, Artem Bityutskiy wrote:
+/* UBI headers must take 64 bytes. The below is a hacky way to ensure this */
+static int __ubi_check_ec_hdr_size[(UBI_EC_HDR_SIZE == 64) - 1]
+__attribute__ ((__unused__));
+static int __ubi_check_ec_hdr_size[(UBI_VID_HDR_SIZE == 64) - 1]
+__attribute__ ((__unused__));
please use BUILD_BUG_ON instead.
+
+static int ubi_attach_mtd_dev(const char *mtd_dev, int vid_hdr_offset,
+ int data_offset);
+static void ubi_destroy_dev(int ubi_num);
Can you reorder the code to avoid all these forward declarations please?
+ /* Attach MTD devices */
+ for (i = 0; i mtd_devs; i++) {
+ struct mtd_dev_param *p = mtd_dev_param[i];
+
+ cond_resched();
+ err = -EINVAL;
This looks very odd.
+module_param_call(mtd, ubi_mtd_param_parse, NULL, NULL, 000);
+MODULE_PARM_DESC(mtd, MTD devices to attach. Parameter format:
+ mtd=name|num[,vid_hdr_offs,data_offs].
+ Multiple \mtd\ parameters may be specified.\n
+ MTD devices may be specified by their number or name.
+ Optional \vid_hdr_offs\ and \data_offs\ parameters
+ specify UBI VID header position and data starting
+ position to be used by UBI.\n
+ Example: mtd=content,1984,2048 mtd=4 - attach MTD device
+ with name content using VID header offset 1984 and data
+ start 2048, and MTD device number 4 using default
+ offsets);
This is a very odd paramater interface. We really don't want drivers to use
module_param_call directly. You probably want various module_param_array calls
instead.
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
