| Message ID | 20090210221710.389c264e@pedra.chehab.org (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
On Tue, 10 Feb 2009, Mauro Carvalho Chehab wrote: > > I did some more testing and the bug disappears in kernel 2.6.25. Also, if I > > just run 'make', then the .config file it produces is fine. I wonder if it > > isn't a bug in menuconfig itself. > > It seems to be a bug at the Kbuild, fixed on Feb, 2008, on this changeset: > commit 587c90616a5b44e6ccfac38e64d4fecee51d588c (attached). > > As explained, after the patch description, the value for the Kconfig var, after > the patch, uses this formula: > > (value && dependency) || select It's odd that the patch is for "fix select in combination with default", yet there is no select used for CX88_DVB. I think what you've done with CX88_MPEG is something that nothing else in has used before, which made use of the behavior introduced by this patch in a new way. > And there there's no select, the value of CONFIG_CX88_MPEG is determined by: > ('y' && dependency) > > The most complex case is when we have CX88 defined as: > CX88 = 'y' > > if both CX88_DVB and CX88_BLACKBIRD are defined as 'm' (or one of them is 'n' > and the other is 'm'), then CX88_MPEG is defined as: > CX88_MPEG = 'm' > > If one of CX88_DVB or CX88_BLACKBIRD is defined as 'y'; then we have: > CX88_MPEG = 'y' > > If both are 'n', we have: > CX88_MPEG = 'n' > > So, it seems that, after commit 587c90616a5b44e6ccfac38e64d4fecee51d588c, > everything is working as expected. We just need to provide a hack at the > out-of-tree build system for kernels that don't have this commit applied. I still think using select is better. What Roman Zippel was talking about was the mess with select and the tuner drivers. I agree that's a mess and there are better ways to do it without using select. But the MPEG module is like a library used by just DVB and BLACKBIRD. It seems like the ideal case for using select. -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 10 Feb 2009 17:20:52 -0800 (PST) Trent Piepho <xyzzy@speakeasy.org> wrote: > On Tue, 10 Feb 2009, Mauro Carvalho Chehab wrote: > > > I did some more testing and the bug disappears in kernel 2.6.25. Also, if I > > > just run 'make', then the .config file it produces is fine. I wonder if it > > > isn't a bug in menuconfig itself. > > > > It seems to be a bug at the Kbuild, fixed on Feb, 2008, on this changeset: > > commit 587c90616a5b44e6ccfac38e64d4fecee51d588c (attached). > > > > As explained, after the patch description, the value for the Kconfig var, after > > the patch, uses this formula: > > > > (value && dependency) || select > > It's odd that the patch is for "fix select in combination with default", > yet there is no select used for CX88_DVB. If you look at the patch code, it fixed the handling for non-visible Kconfig vars. > I think what you've done with CX88_MPEG is something that nothing else in has used before, which made use > of the behavior introduced by this patch in a new way. > > > And there there's no select, the value of CONFIG_CX88_MPEG is determined by: > > ('y' && dependency) > > > > The most complex case is when we have CX88 defined as: > > CX88 = 'y' > > > > if both CX88_DVB and CX88_BLACKBIRD are defined as 'm' (or one of them is 'n' > > and the other is 'm'), then CX88_MPEG is defined as: > > CX88_MPEG = 'm' > > > > If one of CX88_DVB or CX88_BLACKBIRD is defined as 'y'; then we have: > > CX88_MPEG = 'y' > > > > If both are 'n', we have: > > CX88_MPEG = 'n' > > > > So, it seems that, after commit 587c90616a5b44e6ccfac38e64d4fecee51d588c, > > everything is working as expected. We just need to provide a hack at the > > out-of-tree build system for kernels that don't have this commit applied. > > I still think using select is better. What Roman Zippel was talking about > was the mess with select and the tuner drivers. I agree that's a mess and > there are better ways to do it without using select. But the MPEG module > is like a library used by just DVB and BLACKBIRD. It seems like the ideal > case for using select. I can't foresee any case where this logic would fail in the future. Let's suppose that some newer dependencies would be needed. If those dependencies will be properly added at DVB and/or at BLACKBIRD, this logic will still work. There's no possible case where CX88_MPEG would need a dependency that aren't needed by either DVB and/or BLACKBIRD. Also, by using depends on, instead of select, will warrant that CX88_MPEG will have the proper 'y' or 'm' value, depending on the dependencies of CX88_DVB and CX88_BLACKBIRD. It seems that this is exactly what Roman expected to be fixed by changing from "select" to "depends on" with tuners. Cheers, Mauro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 11 Feb 2009, Mauro Carvalho Chehab wrote: > On Tue, 10 Feb 2009 17:20:52 -0800 (PST) > Trent Piepho <xyzzy@speakeasy.org> wrote: > > I still think using select is better. What Roman Zippel was talking about > > was the mess with select and the tuner drivers. I agree that's a mess and > > there are better ways to do it without using select. But the MPEG module > > is like a library used by just DVB and BLACKBIRD. It seems like the ideal > > case for using select. > > I can't foresee any case where this logic would fail in the future. > > Let's suppose that some newer dependencies would be needed. If those > dependencies will be properly added at DVB and/or at BLACKBIRD, this logic will > still work. There's no possible case where CX88_MPEG would need a dependency > that aren't needed by either DVB and/or BLACKBIRD. Also, by using depends on, I think this is the reason select is the better choice here. The only reason select might have a problem is if CX88_MPEG had a dependency that that DVB and BLACKBIRD do not have. But like you said, that isn't going to happen, so there is no problem with select. > instead of select, will warrant that CX88_MPEG will have the proper 'y' or 'm' > value, depending on the dependencies of CX88_DVB and CX88_BLACKBIRD. Using select like I did will result in CX88_MPEG having the proper 'y' or 'm' as well. > It seems that this is exactly what Roman expected to be fixed by changing from > "select" to "depends on" with tuners. The problem with the tuners is that the many tuners each have many different dependencies and are used by multiple drivers. select requires that the drivers using the tuners consider those depedencies and if the tuners change the drivers must also be updated. But with CX88_MPEG we don't have this problem. We also want to be able to manually override tuner selection, which makes things even more complicated for the tuners. -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c index 3929e5b..4a03191 100644 --- a/scripts/kconfig/symbol.c +++ b/scripts/kconfig/symbol.c @@ -298,22 +298,30 @@ void sym_calc_value(struct symbol *sym) if (sym_is_choice_value(sym) && sym->visible == yes) { prop = sym_get_choice_prop(sym); newval.tri = (prop_get_symbol(prop)->curr.val == sym) ? yes : no; - } else if (EXPR_OR(sym->visible, sym->rev_dep.tri) != no) { - sym->flags |= SYMBOL_WRITE; - if (sym_has_value(sym)) - newval.tri = sym->def[S_DEF_USER].tri; - else if (!sym_is_choice(sym)) { - prop = sym_get_default_prop(sym); - if (prop) - newval.tri = expr_calc_value(prop->expr); + } else { + if (sym->visible != no) { + /* if the symbol is visible use the user value + * if available, otherwise try the default value + */ + sym->flags |= SYMBOL_WRITE; + if (sym_has_value(sym)) { + newval.tri = EXPR_AND(sym->def[S_DEF_USER].tri, + sym->visible); + goto calc_newval; + } } - newval.tri = EXPR_OR(EXPR_AND(newval.tri, sym->visible), sym->rev_dep.tri); - } else if (!sym_is_choice(sym)) { - prop = sym_get_default_prop(sym); - if (prop) { + if (sym->rev_dep.tri != no) sym->flags |= SYMBOL_WRITE; - newval.tri = expr_calc_value(prop->expr); + if (!sym_is_choice(sym)) { + prop = sym_get_default_prop(sym); + if (prop) { + sym->flags |= SYMBOL_WRITE; + newval.tri = EXPR_AND(expr_calc_value(prop->expr), + prop->visible.tri); + } } + calc_newval: + newval.tri = EXPR_OR(newval.tri, sym->rev_dep.tri); } if (newval.tri == mod && sym_get_type(sym) == S_BOOLEAN) newval.tri = yes;