| Message ID | 20201119055551.26493-4-vadivel.muruganx.ramuthevar@linux.intel.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v8,1/6] spi: cadence-quadspi: Add QSPI support for Intel LGM SoC | expand |
On 11/19/20 11:25 AM, Ramuthevar,Vadivel MuruganX wrote: > From: Ramuthevar Vadivel Murugan <vadivel.muruganx.ramuthevar@linux.intel.com> > > Add multiple chipselect support for Intel LGM SoCs, > currently QSPI-NOR and QSPI-NAND supported. > > Signed-off-by: Ramuthevar Vadivel Murugan <vadivel.muruganx.ramuthevar@linux.intel.com> > --- > drivers/spi/spi-cadence-quadspi.c | 15 +++++++++++++++ > 1 file changed, 15 insertions(+) > > diff --git a/drivers/spi/spi-cadence-quadspi.c b/drivers/spi/spi-cadence-quadspi.c > index d12b765e87be..337778f75d5d 100644 > --- a/drivers/spi/spi-cadence-quadspi.c > +++ b/drivers/spi/spi-cadence-quadspi.c > @@ -38,6 +38,7 @@ > > /* Capabilities */ > #define CQSPI_SUPPORTS_OCTAL BIT(0) > +#define CQSPI_SUPPORTS_MULTI_CHIPSELECT BIT(1) > > struct cqspi_st; > > @@ -75,6 +76,7 @@ struct cqspi_st { > bool is_decoded_cs; > u32 fifo_depth; > u32 fifo_width; > + u32 num_chipselect; > bool rclk_en; > u32 trigger_address; > u32 wr_delay; > @@ -1049,6 +1051,7 @@ static int cqspi_of_get_flash_pdata(struct platform_device *pdev, > > static int cqspi_of_get_pdata(struct cqspi_st *cqspi) > { > + const struct cqspi_driver_platdata *ddata; Unused variable? > struct device *dev = &cqspi->pdev->dev; > struct device_node *np = dev->of_node; > > @@ -1070,6 +1073,14 @@ static int cqspi_of_get_pdata(struct cqspi_st *cqspi) > return -ENXIO; > } > > + ddata = of_device_get_match_data(dev); > + if (ddata->hwcaps_mask & CQSPI_SUPPORTS_MULTI_CHIPSELECT) { I don't see a need for this flag... Controller by default supports multiple CS. > + if (of_property_read_u32(np, "num-cs", &cqspi->num_chipselect)) { > + dev_err(dev, "couldn't determine number of cs\n"); > + return -ENXIO; > + } > + } > + Entire hunk can be replaced with: if (of_property_read_u32(np, "num-cs", &cqspi->num_chipselect)) cqspi->num_chipselect = CQSPI_MAX_CHIPSELECT; > cqspi->rclk_en = of_property_read_bool(np, "cdns,rclk-en"); > > return 0; > @@ -1302,6 +1313,9 @@ static int cqspi_probe(struct platform_device *pdev) > cqspi->current_cs = -1; > cqspi->sclk = 0; > > + if (ddata->hwcaps_mask & CQSPI_SUPPORTS_MULTI_CHIPSELECT) > + master->num_chipselect = cqspi->num_chipselect; > + And then this becomes: master->num_chipselect = cqspi->num_chipselect; > ret = cqspi_setup_flash(cqspi); > if (ret) { > dev_err(dev, "failed to setup flash parameters %d\n", ret); > @@ -1391,6 +1405,7 @@ static const struct cqspi_driver_platdata am654_ospi = { > }; > > static const struct cqspi_driver_platdata intel_lgm_qspi = { > + .hwcaps_mask = CQSPI_SUPPORTS_MULTI_CHIPSELECT, > .quirks = CQSPI_DISABLE_DAC_MODE, > }; > >
Hi Vignesh, Thank you very much for the review comments... On 19/11/2020 8:57 pm, Vignesh Raghavendra wrote: > > > On 11/19/20 11:25 AM, Ramuthevar,Vadivel MuruganX wrote: >> From: Ramuthevar Vadivel Murugan <vadivel.muruganx.ramuthevar@linux.intel.com> >> >> Add multiple chipselect support for Intel LGM SoCs, >> currently QSPI-NOR and QSPI-NAND supported. >> >> Signed-off-by: Ramuthevar Vadivel Murugan <vadivel.muruganx.ramuthevar@linux.intel.com> >> --- >> drivers/spi/spi-cadence-quadspi.c | 15 +++++++++++++++ >> 1 file changed, 15 insertions(+) >> >> diff --git a/drivers/spi/spi-cadence-quadspi.c b/drivers/spi/spi-cadence-quadspi.c >> index d12b765e87be..337778f75d5d 100644 >> --- a/drivers/spi/spi-cadence-quadspi.c >> +++ b/drivers/spi/spi-cadence-quadspi.c >> @@ -38,6 +38,7 @@ >> >> /* Capabilities */ >> #define CQSPI_SUPPORTS_OCTAL BIT(0) >> +#define CQSPI_SUPPORTS_MULTI_CHIPSELECT BIT(1) >> >> struct cqspi_st; >> >> @@ -75,6 +76,7 @@ struct cqspi_st { >> bool is_decoded_cs; >> u32 fifo_depth; >> u32 fifo_width; >> + u32 num_chipselect; >> bool rclk_en; >> u32 trigger_address; >> u32 wr_delay; >> @@ -1049,6 +1051,7 @@ static int cqspi_of_get_flash_pdata(struct platform_device *pdev, >> >> static int cqspi_of_get_pdata(struct cqspi_st *cqspi) >> { >> + const struct cqspi_driver_platdata *ddata; > > Unused variable? currently used for this check if (ddata->hwcaps_mask & CQSPI_SUPPORTS_MULTI_CHIPSELECT) { next patch will drop it, because the above check to be removed. > >> struct device *dev = &cqspi->pdev->dev; >> struct device_node *np = dev->of_node; >> >> @@ -1070,6 +1073,14 @@ static int cqspi_of_get_pdata(struct cqspi_st *cqspi) >> return -ENXIO; >> } >> >> + ddata = of_device_get_match_data(dev); >> + if (ddata->hwcaps_mask & CQSPI_SUPPORTS_MULTI_CHIPSELECT) { > > I don't see a need for this flag... Controller by default supports > multiple CS. Ok. > >> + if (of_property_read_u32(np, "num-cs", &cqspi->num_chipselect)) { >> + dev_err(dev, "couldn't determine number of cs\n"); >> + return -ENXIO; >> + } >> + } >> + > > > Entire hunk can be replaced with: > > if (of_property_read_u32(np, "num-cs", &cqspi->num_chipselect)) > cqspi->num_chipselect = CQSPI_MAX_CHIPSELECT; Noted, thanks! > > >> cqspi->rclk_en = of_property_read_bool(np, "cdns,rclk-en"); >> >> return 0; >> @@ -1302,6 +1313,9 @@ static int cqspi_probe(struct platform_device *pdev) >> cqspi->current_cs = -1; >> cqspi->sclk = 0; >> >> + if (ddata->hwcaps_mask & CQSPI_SUPPORTS_MULTI_CHIPSELECT) >> + master->num_chipselect = cqspi->num_chipselect; >> + > > And then this becomes: > master->num_chipselect = cqspi->num_chipselect; Ok, Noted. > >> ret = cqspi_setup_flash(cqspi); >> if (ret) { >> dev_err(dev, "failed to setup flash parameters %d\n", ret); >> @@ -1391,6 +1405,7 @@ static const struct cqspi_driver_platdata am654_ospi = { >> }; >> >> static const struct cqspi_driver_platdata intel_lgm_qspi = { >> + .hwcaps_mask = CQSPI_SUPPORTS_MULTI_CHIPSELECT, >> .quirks = CQSPI_DISABLE_DAC_MODE, >> }; >> >>
diff --git a/drivers/spi/spi-cadence-quadspi.c b/drivers/spi/spi-cadence-quadspi.c index d12b765e87be..337778f75d5d 100644 --- a/drivers/spi/spi-cadence-quadspi.c +++ b/drivers/spi/spi-cadence-quadspi.c @@ -38,6 +38,7 @@ /* Capabilities */ #define CQSPI_SUPPORTS_OCTAL BIT(0) +#define CQSPI_SUPPORTS_MULTI_CHIPSELECT BIT(1) struct cqspi_st; @@ -75,6 +76,7 @@ struct cqspi_st { bool is_decoded_cs; u32 fifo_depth; u32 fifo_width; + u32 num_chipselect; bool rclk_en; u32 trigger_address; u32 wr_delay; @@ -1049,6 +1051,7 @@ static int cqspi_of_get_flash_pdata(struct platform_device *pdev, static int cqspi_of_get_pdata(struct cqspi_st *cqspi) { + const struct cqspi_driver_platdata *ddata; struct device *dev = &cqspi->pdev->dev; struct device_node *np = dev->of_node; @@ -1070,6 +1073,14 @@ static int cqspi_of_get_pdata(struct cqspi_st *cqspi) return -ENXIO; } + ddata = of_device_get_match_data(dev); + if (ddata->hwcaps_mask & CQSPI_SUPPORTS_MULTI_CHIPSELECT) { + if (of_property_read_u32(np, "num-cs", &cqspi->num_chipselect)) { + dev_err(dev, "couldn't determine number of cs\n"); + return -ENXIO; + } + } + cqspi->rclk_en = of_property_read_bool(np, "cdns,rclk-en"); return 0; @@ -1302,6 +1313,9 @@ static int cqspi_probe(struct platform_device *pdev) cqspi->current_cs = -1; cqspi->sclk = 0; + if (ddata->hwcaps_mask & CQSPI_SUPPORTS_MULTI_CHIPSELECT) + master->num_chipselect = cqspi->num_chipselect; + ret = cqspi_setup_flash(cqspi); if (ret) { dev_err(dev, "failed to setup flash parameters %d\n", ret); @@ -1391,6 +1405,7 @@ static const struct cqspi_driver_platdata am654_ospi = { }; static const struct cqspi_driver_platdata intel_lgm_qspi = { + .hwcaps_mask = CQSPI_SUPPORTS_MULTI_CHIPSELECT, .quirks = CQSPI_DISABLE_DAC_MODE, };