On Tuesday, July 28, 2015 at 07:38:03 PM, Graham Moore wrote: DTTO here.
Thanks a lot for working on the driver though -- would you like me to continue reviewing or just take over please ? > Signed-off-by: Graham Moore <[email protected]> > --- > V2: use NULL instead of modalias in spi_nor_scan call > V3: Use existing property is-decoded-cs instead of creating duplicate. > V4: Support Micron quad mode by snooping command stream for EVCR command > and subsequently configuring Cadence controller for quad mode. > V5: Clean up sparse and smatch complaints. Remove snooping of Micron > quad mode. Add comment on XIP mode bit and dummy clock cycles. Set > up SRAM partition at 1:1 during init. > V6: Remove dts patch that was included by mistake. Incorporate Vikas's > comments regarding fifo width, SRAM partition setting, and trigger > address. Trigger address was added as an unsigned int, as it is not > an IO resource per se, and does not need to be mapped. Also add > Marek Vasut's workaround for picking up OF properties on subnodes. I don't think that's the correct thing to do with the OF nodes btw ;-) [...] > +#define CQSPI_REG_IS_IDLE(base) > \ > + ((readl(base + CQSPI_REG_CONFIG) >> \ > + CQSPI_REG_CONFIG_IDLE_LSB) & 0x1) This should be a function (if needed). > +#define CQSPI_GET_RD_SRAM_LEVEL(reg_base) \ > + (((readl(reg_base + CQSPI_REG_SDRAMLEVEL)) >> \ > + CQSPI_REG_SDRAMLEVEL_RD_LSB) & CQSPI_REG_SDRAMLEVEL_RD_MASK) DTTO. > +static unsigned int cqspi_init_timeout(unsigned long timeout_in_ms) > +{ > + return jiffies + msecs_to_jiffies(timeout_in_ms); > +} This in turn might better be wrapped into the code. > +static unsigned int cqspi_check_timeout(unsigned long timeout) > +{ > + return time_before(jiffies, timeout); > +} DTTO. [...] > +static int cqspi_indirect_read_setup(struct spi_nor *nor, > + unsigned int from_addr) > +{ > + unsigned int reg; > + unsigned int dummy_clk = 0; > + struct cqspi_st *cqspi = nor->priv; > + void __iomem *reg_base = cqspi->iobase; > + > + writel(cqspi->trigger_address, > + reg_base + CQSPI_REG_INDIRECTTRIGGER); > + writel(from_addr, reg_base + CQSPI_REG_INDIRECTRDSTARTADDR); > + > + reg = nor->read_opcode << CQSPI_REG_RD_INSTR_OPCODE_LSB; > + reg |= cqspi_calc_rdreg(nor, nor->read_opcode); > + > + /* Setup dummy clock cycles */ > +#define CQSPI_SUPPORT_XIP_CHIPS What's this supposed to do ? Should this be compile-time config or should this be in DT ? > +#ifdef CQSPI_SUPPORT_XIP_CHIPS > + /* > + * Set mode bits high to ensure chip doesn't enter XIP. > + * This results in an extra 8 dummy clocks so > + * we must account for them. > + */ > + writel(0xFF, reg_base + CQSPI_REG_MODE_BIT); > + reg |= (1 << CQSPI_REG_RD_INSTR_MODE_EN_LSB); > + if (nor->read_dummy >= 8) > + dummy_clk = nor->read_dummy - 8; > + else > + dummy_clk = 0; > +#else > + dummy_clk = nor->read_dummy; > +#endif > + reg |= (dummy_clk & CQSPI_REG_RD_INSTR_DUMMY_MASK) > + << CQSPI_REG_RD_INSTR_DUMMY_LSB; > + > + writel(reg, reg_base + CQSPI_REG_RD_INSTR); > + > + /* Set address width */ > + reg = readl(reg_base + CQSPI_REG_SIZE); > + reg &= ~CQSPI_REG_SIZE_ADDRESS_MASK; > + reg |= (nor->addr_width - 1); > + writel(reg, reg_base + CQSPI_REG_SIZE); > + return 0; > +} [...] > +static void cqspi_switch_cs(struct cqspi_st *cqspi, unsigned int cs) > +{ > + unsigned int reg; > + struct cqspi_flash_pdata *f_pdata = &cqspi->f_pdata[cs]; > + void __iomem *iobase = cqspi->iobase; > + struct spi_nor *nor = &f_pdata->nor; > + > + cqspi_controller_disable(cqspi); > + > + /* configure page size and block size. */ > + reg = readl(iobase + CQSPI_REG_SIZE); > + reg &= ~(CQSPI_REG_SIZE_PAGE_MASK << CQSPI_REG_SIZE_PAGE_LSB); > + reg &= ~(CQSPI_REG_SIZE_BLOCK_MASK << CQSPI_REG_SIZE_BLOCK_LSB); > + reg |= (nor->page_size << CQSPI_REG_SIZE_PAGE_LSB); > + reg |= (ilog2(nor->mtd->erasesize) << CQSPI_REG_SIZE_BLOCK_LSB); > + reg &= ~CQSPI_REG_SIZE_ADDRESS_MASK; > + reg |= (nor->addr_width - 1); This could use some reordering -- probably clear bits first, set second. > + writel(reg, iobase + CQSPI_REG_SIZE); > + > + /* configure the chip select */ > + cqspi_chipselect(cqspi, cs, cqspi->is_decoded_cs); > + > + cqspi_controller_enable(cqspi); > +} [...] > +static int cqspi_probe(struct platform_device *pdev) > +{ > + struct device_node *np = pdev->dev.of_node; > + struct mtd_part_parser_data ppdata; > + struct device *dev = &pdev->dev; > + struct cqspi_st *cqspi; > + struct spi_nor *nor; > + struct mtd_info *mtd; > + struct resource *res; > + struct resource *res_ahb; > + int ret; > + int irq; > + > + cqspi = devm_kzalloc(dev, sizeof(*cqspi), GFP_KERNEL); > + if (!cqspi) > + return -ENOMEM; > + > + cqspi->pdev = pdev; > + platform_set_drvdata(pdev, cqspi); > + > + ret = cqspi_of_get_pdata(pdev); > + if (ret) { > + dev_err(dev, "Get platform data failed.\n"); > + return -ENODEV; > + } > + > + cqspi->clk = devm_clk_get(dev, NULL); > + if (IS_ERR(cqspi->clk)) { > + dev_err(dev, "cannot get qspi clk\n"); > + ret = PTR_ERR(cqspi->clk); > + goto probe_failed; > + } > + cqspi->master_ref_clk_hz = clk_get_rate(cqspi->clk); > + > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + cqspi->iobase = devm_ioremap_resource(dev, res); > + if (IS_ERR(cqspi->iobase)) { > + dev_err(dev, "devm_ioremap_resource res 0 failed\n"); > + ret = PTR_ERR(cqspi->iobase); > + goto probe_failed; > + } > + > + res_ahb = platform_get_resource(pdev, IORESOURCE_MEM, 1); > + cqspi->ahb_phy_addr = res_ahb->start; > + cqspi->ahb_base = devm_ioremap_resource(dev, res_ahb); > + if (IS_ERR(cqspi->ahb_base)) { > + dev_err(dev, "devm_ioremap_resource res 1 failed\n"); > + ret = PTR_ERR(cqspi->ahb_base); > + goto probe_failed; > + } > + > + init_completion(&cqspi->transfer_complete); > + > + irq = platform_get_irq(pdev, 0); > + if (irq < 0) { > + dev_err(dev, "platform_get_irq failed\n"); > + ret = -ENXIO; > + goto probe_failed; > + } > + ret = devm_request_irq(dev, irq, > + cqspi_irq_handler, 0, pdev->name, cqspi); > + if (ret) { > + dev_err(dev, "devm_request_irq failed\n"); > + goto probe_failed; > + } > + > + cqspi_wait_idle(cqspi); > + cqspi_controller_init(cqspi); > + cqspi->current_cs = -1; > + cqspi->sclk = 0; > + > + /* Get flash device data */ > + for_each_available_child_of_node(dev->of_node, np) { > + unsigned int cs; > + struct cqspi_flash_pdata *f_pdata; > + > + if (of_property_read_u32(np, "reg", &cs)) { > + dev_err(dev, "couldn't determine chip select\n"); > + return -ENXIO; > + } > + if (cs >= CQSPI_MAX_CHIPSELECT) { > + dev_err(dev, "chip select %d out of range\n", cs); > + return -ENXIO; > + } > + f_pdata = &cqspi->f_pdata[cs]; > + > + ret = cqspi_of_get_flash_pdata(pdev, f_pdata, np); > + if (ret) > + goto probe_failed; > + > + nor = &f_pdata->nor; > + mtd = &f_pdata->mtd; > + > + nor->mtd = mtd; > + nor->dev = dev; > + nor->priv = cqspi; > + mtd->priv = nor; > + > + nor->read_reg = cqspi_read_reg; > + nor->write_reg = cqspi_write_reg; > + nor->read = cqspi_read; > + nor->write = cqspi_write; > + nor->erase = cqspi_erase; > + > + nor->prepare = cqspi_prep; > + > + /* > + * Here is a 'nasty hack' from Marek Vasut to pick > + * up OF properties from flash device subnode. > + */ > + nor->dev->of_node = np; WARNING: HERE IS A HACK! This is true indeed, can someone please give feedback on how to do this right ? > + ret = spi_nor_scan(nor, NULL, SPI_NOR_QUAD); > + if (ret) > + goto probe_failed; > + > + if (nor->read_dummy > CQSPI_DUMMY_CLKS_MAX) > + nor->read_dummy = CQSPI_DUMMY_CLKS_MAX; > + > + ppdata.of_node = np; > + ret = mtd_device_parse_register(mtd, NULL, &ppdata, NULL, 0); > + if (ret) > + goto probe_failed; > + } [...] -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
