Messages in this thread Patch in this message | | | Subject | Re: [PATCH v8 3/6] spi: cadence-quadspi: Add multi-chipselect support for Intel LGM SoC | From | "Ramuthevar, Vadivel MuruganX" <> | Date | Fri, 20 Nov 2020 08:29:51 +0800 |
| |
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, >> }; >> >>
| |