| Message ID | 1445971076-17270-14-git-send-email-jteki@openedev.com |
|---|---|
| State | Superseded |
| Delegated to: | Jagannadha Sutradharudu Teki |
| Headers | show |
On 10/27/2015 11:37 AM, Jagan Teki wrote: > spi_flash_probe_tail code looks not in proper shape > to add more functionalities. hence refactorized > so-that it's more readable and hence we may extend > more functionalies to it. > > Signed-off-by: Jagan Teki <jteki@openedev.com> > --- > drivers/mtd/spi/sf_probe.c | 22 ++++++++++++++-------- > 1 file changed, 14 insertions(+), 8 deletions(-) > > diff --git a/drivers/mtd/spi/sf_probe.c b/drivers/mtd/spi/sf_probe.c > index 319b7e6..87ac33e 100644 > --- a/drivers/mtd/spi/sf_probe.c > +++ b/drivers/mtd/spi/sf_probe.c > @@ -123,15 +123,12 @@ int spi_flash_std_erase(struct udevice *dev, u32 offset, size_t len) > > int spi_flash_std_probe(struct udevice *dev) > { > - struct dm_spi_slave_platdata *plat = dev_get_parent_platdata(dev); > + struct spi_flash *flash = dev_get_uclass_priv(dev); > struct spi_slave *slave = dev_get_parentdata(dev); > - struct spi_flash *flash; > int ret; > > - debug("%s: slave=%p, cs=%d\n", __func__, slave, plat->cs); > - > - flash = dev_get_uclass_priv(dev); > flash->dev = dev; > + flash->spi = slave; > > /* Claim spi bus */ > ret = spi_claim_bus(slave); > @@ -140,17 +137,26 @@ int spi_flash_std_probe(struct udevice *dev) > return ret; > } > > - ret = spi_flash_scan(slave, flash); > + ret = spi_flash_scan(flash); Is this bisectable ? It doesn't look like it. > if (ret) { > ret = -EINVAL; > - goto err_read_id; > + goto err_scan; > } > > #ifdef CONFIG_SPI_FLASH_MTD > ret = spi_flash_mtd_register(flash); > + if (ret) { > + printf("SF: failed to register mtd device: %d\n", ret); > + goto err_mtd; > + } > #endif > + return ret; > > -err_read_id: > +#ifdef CONFIG_SPI_FLASH_MTD > +err_mtd: > + spi_free_slave(slave); > +#endif > +err_scan: > spi_release_bus(slave); > return ret; > } >
On 28 October 2015 at 00:50, Troy Kisky <troy.kisky@boundarydevices.com> wrote: > On 10/27/2015 11:37 AM, Jagan Teki wrote: >> spi_flash_probe_tail code looks not in proper shape >> to add more functionalities. hence refactorized >> so-that it's more readable and hence we may extend >> more functionalies to it. >> >> Signed-off-by: Jagan Teki <jteki@openedev.com> >> --- >> drivers/mtd/spi/sf_probe.c | 22 ++++++++++++++-------- >> 1 file changed, 14 insertions(+), 8 deletions(-) >> >> diff --git a/drivers/mtd/spi/sf_probe.c b/drivers/mtd/spi/sf_probe.c >> index 319b7e6..87ac33e 100644 >> --- a/drivers/mtd/spi/sf_probe.c >> +++ b/drivers/mtd/spi/sf_probe.c >> @@ -123,15 +123,12 @@ int spi_flash_std_erase(struct udevice *dev, u32 offset, size_t len) >> >> int spi_flash_std_probe(struct udevice *dev) >> { >> - struct dm_spi_slave_platdata *plat = dev_get_parent_platdata(dev); >> + struct spi_flash *flash = dev_get_uclass_priv(dev); >> struct spi_slave *slave = dev_get_parentdata(dev); >> - struct spi_flash *flash; >> int ret; >> >> - debug("%s: slave=%p, cs=%d\n", __func__, slave, plat->cs); >> - >> - flash = dev_get_uclass_priv(dev); >> flash->dev = dev; >> + flash->spi = slave; >> >> /* Claim spi bus */ >> ret = spi_claim_bus(slave); >> @@ -140,17 +137,26 @@ int spi_flash_std_probe(struct udevice *dev) >> return ret; >> } >> >> - ret = spi_flash_scan(slave, flash); >> + ret = spi_flash_scan(flash); > > > > Is this bisectable ? It doesn't look like it. What you mean bisectable here? This commit re-factorize the code in accordance with changes introduced in v5 13/14 on dm-sf front. > > >> if (ret) { >> ret = -EINVAL; >> - goto err_read_id; >> + goto err_scan; >> } >> >> #ifdef CONFIG_SPI_FLASH_MTD >> ret = spi_flash_mtd_register(flash); >> + if (ret) { >> + printf("SF: failed to register mtd device: %d\n", ret); >> + goto err_mtd; >> + } >> #endif >> + return ret; >> >> -err_read_id: >> +#ifdef CONFIG_SPI_FLASH_MTD >> +err_mtd: >> + spi_free_slave(slave); >> +#endif >> +err_scan: >> spi_release_bus(slave); >> return ret; >> }
Hi Jagan, On Wed, Oct 28, 2015 at 1:18 PM, Jagan Teki <jteki@openedev.com> wrote: > On 28 October 2015 at 00:50, Troy Kisky <troy.kisky@boundarydevices.com> wrote: >> On 10/27/2015 11:37 AM, Jagan Teki wrote: >>> spi_flash_probe_tail code looks not in proper shape >>> to add more functionalities. hence refactorized >>> so-that it's more readable and hence we may extend >>> more functionalies to it. >>> >>> Signed-off-by: Jagan Teki <jteki@openedev.com> >>> --- >>> drivers/mtd/spi/sf_probe.c | 22 ++++++++++++++-------- >>> 1 file changed, 14 insertions(+), 8 deletions(-) >>> >>> diff --git a/drivers/mtd/spi/sf_probe.c b/drivers/mtd/spi/sf_probe.c >>> index 319b7e6..87ac33e 100644 >>> --- a/drivers/mtd/spi/sf_probe.c >>> +++ b/drivers/mtd/spi/sf_probe.c >>> @@ -123,15 +123,12 @@ int spi_flash_std_erase(struct udevice *dev, u32 offset, size_t len) >>> >>> int spi_flash_std_probe(struct udevice *dev) >>> { >>> - struct dm_spi_slave_platdata *plat = dev_get_parent_platdata(dev); >>> + struct spi_flash *flash = dev_get_uclass_priv(dev); >>> struct spi_slave *slave = dev_get_parentdata(dev); >>> - struct spi_flash *flash; >>> int ret; >>> >>> - debug("%s: slave=%p, cs=%d\n", __func__, slave, plat->cs); >>> - >>> - flash = dev_get_uclass_priv(dev); >>> flash->dev = dev; >>> + flash->spi = slave; >>> >>> /* Claim spi bus */ >>> ret = spi_claim_bus(slave); >>> @@ -140,17 +137,26 @@ int spi_flash_std_probe(struct udevice *dev) >>> return ret; >>> } >>> >>> - ret = spi_flash_scan(slave, flash); >>> + ret = spi_flash_scan(flash); >> >> >> >> Is this bisectable ? It doesn't look like it. > > What you mean bisectable here? This commit re-factorize the code in > accordance with changes introduced in v5 13/14 on dm-sf front. > We should make sure every commit can build successfully, aka bisectable via 'git bisect'. It looks like you changed the spi_flash_scan() function signature somewhere else, but not in this commit, hence the question bisectable? Regards, Bin
On 28 October 2015 at 11:01, Bin Meng <bmeng.cn@gmail.com> wrote: > Hi Jagan, > > On Wed, Oct 28, 2015 at 1:18 PM, Jagan Teki <jteki@openedev.com> wrote: >> On 28 October 2015 at 00:50, Troy Kisky <troy.kisky@boundarydevices.com> wrote: >>> On 10/27/2015 11:37 AM, Jagan Teki wrote: >>>> spi_flash_probe_tail code looks not in proper shape >>>> to add more functionalities. hence refactorized >>>> so-that it's more readable and hence we may extend >>>> more functionalies to it. Thanks Bin, for the info I will move spi_flash_scan change back to 13/14 patch. >>>> >>>> Signed-off-by: Jagan Teki <jteki@openedev.com> >>>> --- >>>> drivers/mtd/spi/sf_probe.c | 22 ++++++++++++++-------- >>>> 1 file changed, 14 insertions(+), 8 deletions(-) >>>> >>>> diff --git a/drivers/mtd/spi/sf_probe.c b/drivers/mtd/spi/sf_probe.c >>>> index 319b7e6..87ac33e 100644 >>>> --- a/drivers/mtd/spi/sf_probe.c >>>> +++ b/drivers/mtd/spi/sf_probe.c >>>> @@ -123,15 +123,12 @@ int spi_flash_std_erase(struct udevice *dev, u32 offset, size_t len) >>>> >>>> int spi_flash_std_probe(struct udevice *dev) >>>> { >>>> - struct dm_spi_slave_platdata *plat = dev_get_parent_platdata(dev); >>>> + struct spi_flash *flash = dev_get_uclass_priv(dev); >>>> struct spi_slave *slave = dev_get_parentdata(dev); >>>> - struct spi_flash *flash; >>>> int ret; >>>> >>>> - debug("%s: slave=%p, cs=%d\n", __func__, slave, plat->cs); >>>> - >>>> - flash = dev_get_uclass_priv(dev); >>>> flash->dev = dev; >>>> + flash->spi = slave; >>>> >>>> /* Claim spi bus */ >>>> ret = spi_claim_bus(slave); >>>> @@ -140,17 +137,26 @@ int spi_flash_std_probe(struct udevice *dev) >>>> return ret; >>>> } >>>> >>>> - ret = spi_flash_scan(slave, flash); >>>> + ret = spi_flash_scan(flash); >>> >>> >>> >>> Is this bisectable ? It doesn't look like it. >> >> What you mean bisectable here? This commit re-factorize the code in >> accordance with changes introduced in v5 13/14 on dm-sf front. >> > > We should make sure every commit can build successfully, aka > bisectable via 'git bisect'. It looks like you changed the > spi_flash_scan() function signature somewhere else, but not in this > commit, hence the question bisectable? > > Regards, > Bin > _______________________________________________ > U-Boot mailing list > U-Boot@lists.denx.de > http://lists.denx.de/mailman/listinfo/u-boot
diff --git a/drivers/mtd/spi/sf_probe.c b/drivers/mtd/spi/sf_probe.c index 319b7e6..87ac33e 100644 --- a/drivers/mtd/spi/sf_probe.c +++ b/drivers/mtd/spi/sf_probe.c @@ -123,15 +123,12 @@ int spi_flash_std_erase(struct udevice *dev, u32 offset, size_t len) int spi_flash_std_probe(struct udevice *dev) { - struct dm_spi_slave_platdata *plat = dev_get_parent_platdata(dev); + struct spi_flash *flash = dev_get_uclass_priv(dev); struct spi_slave *slave = dev_get_parentdata(dev); - struct spi_flash *flash; int ret; - debug("%s: slave=%p, cs=%d\n", __func__, slave, plat->cs); - - flash = dev_get_uclass_priv(dev); flash->dev = dev; + flash->spi = slave; /* Claim spi bus */ ret = spi_claim_bus(slave); @@ -140,17 +137,26 @@ int spi_flash_std_probe(struct udevice *dev) return ret; } - ret = spi_flash_scan(slave, flash); + ret = spi_flash_scan(flash); if (ret) { ret = -EINVAL; - goto err_read_id; + goto err_scan; } #ifdef CONFIG_SPI_FLASH_MTD ret = spi_flash_mtd_register(flash); + if (ret) { + printf("SF: failed to register mtd device: %d\n", ret); + goto err_mtd; + } #endif + return ret; -err_read_id: +#ifdef CONFIG_SPI_FLASH_MTD +err_mtd: + spi_free_slave(slave); +#endif +err_scan: spi_release_bus(slave); return ret; }
spi_flash_probe_tail code looks not in proper shape to add more functionalities. hence refactorized so-that it's more readable and hence we may extend more functionalies to it. Signed-off-by: Jagan Teki <jteki@openedev.com> --- drivers/mtd/spi/sf_probe.c | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-)