| Message ID | 1386430787-2962-1-git-send-email-ezequiel.garcia@free-electrons.com |
|---|---|
| State | Superseded |
| Headers | show |
Ezequiel, I would prefer to call this a 'partial revert of c0f3b8643a6f ...' On Sat, Dec 07, 2013 at 12:39:47PM -0300, Ezequiel Garcia wrote: > Currently the "armada370-nand" compatible support is not complete, > and it was mistake to add it. Instead of completely removing the compatible, > let's just disable it until all the needed infrastructure is in place. > > Cc: Emilio López <emilio@elopez.com.ar> > Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com> > --- > The compatible was added in: > > commit c0f3b8643a6fa2461d70760ec49d21d2b031d611 > Author: Ezequiel Garcia <ezequiel.garcia@free-electrons.com> > Date: Sat Aug 10 16:34:52 2013 -0300 > > mtd: nand: pxa3xx: Introduce 'marvell,armada370-nand' compatible string And add this as a oneline in the commit description. > > Which means the fix should be applied in v3.12 and v3.13. > > On Emilio's suggestion, I've opted for the disabling, given all the required > support is already queued for v3.14. I'll push a patch removing the #if 0 > on top of current l2-mtd.git. > > Brian: Do you think this is OK to be pushed now for v3.13? > > drivers/mtd/nand/pxa3xx_nand.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c > index 3d143fe..2c5066f 100644 > --- a/drivers/mtd/nand/pxa3xx_nand.c > +++ b/drivers/mtd/nand/pxa3xx_nand.c > @@ -333,10 +333,16 @@ static struct of_device_id pxa3xx_nand_dt_ids[] = { > .compatible = "marvell,pxa3xx-nand", > .data = (void *)PXA3XX_NAND_VARIANT_PXA, > }, > +/* > + * Currently, the armada370 support is incomplete and can cause the > + * system to crash. Disable it until all the infrastructure is in place. > + */ > +#if 0 > { > .compatible = "marvell,armada370-nand", > .data = (void *)PXA3XX_NAND_VARIANT_ARMADA370, > }, > +#endif I prefer to just remove the lines. thx, Jason.
On Sat, Dec 07, 2013 at 06:23:15PM -0500, Jason Cooper wrote: > > +/* > > + * Currently, the armada370 support is incomplete and can cause the > > + * system to crash. Disable it until all the infrastructure is in place. > > + */ > > +#if 0 > > { > > .compatible = "marvell,armada370-nand", > > .data = (void *)PXA3XX_NAND_VARIANT_ARMADA370, > > }, > > +#endif > > I prefer to just remove the lines. > I'm actually fine either way. Did both (disable and remove) but since all the code is already sitting and waiting for v3.14 thought it might be cleaner to just "disable" it. Brian (given you're going to take the patch): what do you think? If we agree to remove the lines, shall we also revert the binding documentation, removing the compatible from there as well?
On Sat, Dec 07, 2013 at 10:35:26PM -0300, Ezequiel Garcia wrote: > If we agree to remove the lines, shall we also revert the binding > documentation, removing the compatible from there as well? I think this is unnecessary. We're trying to prevent breaking bisection. No one in their right mind should be developing (and referring to docs) at a random commit. As long as the docs are correct, leave them alone. thx, Jason.
Hi Ezequiel, On Sat, Dec 07, 2013 at 08:40:49PM -0500, Jason Cooper wrote: > On Sat, Dec 07, 2013 at 10:35:26PM -0300, Ezequiel Garcia wrote: > > If we agree to remove the lines, shall we also revert the binding > > documentation, removing the compatible from there as well? > > I think this is unnecessary. We're trying to prevent breaking > bisection. No one in their right mind should be developing (and > referring to docs) at a random commit. As long as the docs are correct, > leave them alone. I agree, we don't need to drop all the docs, just the lines with the 'compatible' property. And #if 0 is a no-go; let's just delete the lines. Yes, the docs will appear in 3.12 and 3.13 even though the driver support won't be there until 3.14, but that's OK IMO. Do you think this should go to 3.12.y stable? If so, please add the appropriate Cc tag. BTW, you moved these lines around in 3.13 so far, so the patch probably won't apply 100% clean. Can you refresh this patch according to Jason's comments and resend? Brian
On Mon, Dec 09, 2013 at 12:46:07PM -0800, Brian Norris wrote: > Hi Ezequiel, > > On Sat, Dec 07, 2013 at 08:40:49PM -0500, Jason Cooper wrote: > > On Sat, Dec 07, 2013 at 10:35:26PM -0300, Ezequiel Garcia wrote: > > > If we agree to remove the lines, shall we also revert the binding > > > documentation, removing the compatible from there as well? > > > > I think this is unnecessary. We're trying to prevent breaking > > bisection. No one in their right mind should be developing (and > > referring to docs) at a random commit. As long as the docs are correct, > > leave them alone. > > I agree, we don't need to drop all the docs, just the lines with the > 'compatible' property. And #if 0 is a no-go; let's just delete the > lines. > > Yes, the docs will appear in 3.12 and 3.13 even though the driver > support won't be there until 3.14, but that's OK IMO. > > Do you think this should go to 3.12.y stable? If so, please add the > appropriate Cc tag. BTW, you moved these lines around in 3.13 so far, > so the patch probably won't apply 100% clean. > > Can you refresh this patch according to Jason's comments and resend? > Sure!
diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c index 3d143fe..2c5066f 100644 --- a/drivers/mtd/nand/pxa3xx_nand.c +++ b/drivers/mtd/nand/pxa3xx_nand.c @@ -333,10 +333,16 @@ static struct of_device_id pxa3xx_nand_dt_ids[] = { .compatible = "marvell,pxa3xx-nand", .data = (void *)PXA3XX_NAND_VARIANT_PXA, }, +/* + * Currently, the armada370 support is incomplete and can cause the + * system to crash. Disable it until all the infrastructure is in place. + */ +#if 0 { .compatible = "marvell,armada370-nand", .data = (void *)PXA3XX_NAND_VARIANT_ARMADA370, }, +#endif {} }; MODULE_DEVICE_TABLE(of, pxa3xx_nand_dt_ids);
Currently the "armada370-nand" compatible support is not complete, and it was mistake to add it. Instead of completely removing the compatible, let's just disable it until all the needed infrastructure is in place. Cc: Emilio López <emilio@elopez.com.ar> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com> --- The compatible was added in: commit c0f3b8643a6fa2461d70760ec49d21d2b031d611 Author: Ezequiel Garcia <ezequiel.garcia@free-electrons.com> Date: Sat Aug 10 16:34:52 2013 -0300 mtd: nand: pxa3xx: Introduce 'marvell,armada370-nand' compatible string Which means the fix should be applied in v3.12 and v3.13. On Emilio's suggestion, I've opted for the disabling, given all the required support is already queued for v3.14. I'll push a patch removing the #if 0 on top of current l2-mtd.git. Brian: Do you think this is OK to be pushed now for v3.13? drivers/mtd/nand/pxa3xx_nand.c | 6 ++++++ 1 file changed, 6 insertions(+)