| Message ID | 1461744756-31481-2-git-send-email-yao.yuan@freescale.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Apr 27, 2016 at 04:12:35PM +0800, Yuan Yao wrote: > At first we use regmap to cover this issue. > But the regmap is designed too large and complex. > The issue for regmap often effect the DSPI's stability. I'm not convinced that open coding things is going to make life much easier here. If there's problems in the underlying code then fixing those seems like a more generally useful approach to things. What are the actual problems this is intended to fix?
On Wed, Apr 27, 2016 at 3:12 AM, Yuan Yao <yao.yuan@freescale.com> wrote: > From: Yuan Yao <yao.yuan@nxp.com> > > The qSPI controller's endian is independent of the CPU core's endian. DSPI? > > For eg, Core on NXP LS1043A SoC is little endian but DSPI is big endian > whereas Core on LS2080A SoC is little endian and DSPI also is little > endian on the same core. It's not correct to say the SoC is little endian for LS1043 or LS2080. Not all devices on the SoC are sharing the same endianness and the core endianness is configurable. > > At first we use regmap to cover this issue. > But the regmap is designed too large and complex. > The issue for regmap often effect the DSPI's stability. This is not stability issue but functional issue. > > DSPI driver just only need a effective way to R/W the controller register > with BE or LE mode. > > So it's better to packaging a sample function. > This will make the DSPI driver more stability and high effective. > > Signed-off-by: Yuan Yao <yao.yuan@nxp.com> > --- > drivers/spi/spi-fsl-dspi.c | 99 +++++++++++++++++++++++++++------------------- > 1 file changed, 59 insertions(+), 40 deletions(-) > > diff --git a/drivers/spi/spi-fsl-dspi.c b/drivers/spi/spi-fsl-dspi.c > index 9e9dadb..9a61572 100644 > --- a/drivers/spi/spi-fsl-dspi.c > +++ b/drivers/spi/spi-fsl-dspi.c > @@ -27,7 +27,6 @@ > #include <linux/pinctrl/consumer.h> > #include <linux/platform_device.h> > #include <linux/pm_runtime.h> > -#include <linux/regmap.h> > #include <linux/sched.h> > #include <linux/spi/spi.h> > #include <linux/spi/spi_bitbang.h> > @@ -143,7 +142,7 @@ struct fsl_dspi { > struct spi_master *master; > struct platform_device *pdev; > > - struct regmap *regmap; > + void __iomem *iobase; > int irq; > struct clk *clk; > > @@ -165,13 +164,46 @@ struct fsl_dspi { > u32 waitflags; > > u32 spi_tcnt; > + bool big_endian; > }; > > +/* > + * R/W functions for big- or little-endian registers: > + * The qSPI controller's endian is independent of the CPU core's endian. DSPI? > + */ > +static void dspi_writel(struct fsl_dspi *d, u32 val, u32 offset) > +{ > + if (d->big_endian) > + iowrite32be(val, d->iobase + offset); > + else > + iowrite32(val, d->iobase + offset); > +} > + > +static u32 dspi_readl(struct fsl_dspi *d, u32 offset) > +{ > + if (d->big_endian) > + return ioread32be(d->iobase + offset); > + else > + return ioread32(d->iobase + offset); > +} > + > +static void dspi_updatel(struct fsl_dspi *d, u32 mask, u32 val, u32 offset) > +{ > + u32 tmp, orig; > + > + orig = dspi_readl(d, offset); > + > + tmp = orig & ~mask; > + tmp |= val & mask; > + > + dspi_writel(d, tmp, offset); > +} > + > static inline int is_double_byte_mode(struct fsl_dspi *dspi) > { > unsigned int val; > > - regmap_read(dspi->regmap, SPI_CTAR(0), &val); > + val = dspi_readl(dspi, SPI_CTAR(0)); > > return ((val & SPI_FRAME_BITS_MASK) == SPI_FRAME_BITS(8)) ? 0 : 1; > } > @@ -270,7 +302,7 @@ static void dspi_data_from_popr(struct fsl_dspi *dspi, int rx_word) > u16 d; > unsigned int val; > > - regmap_read(dspi->regmap, SPI_POPR, &val); > + val = dspi_readl(dspi, SPI_POPR); > d = SPI_POPR_RXDATA(val); > > if (!(dspi->dataflags & TRAN_STATE_RX_VOID)) > @@ -294,8 +326,8 @@ static int dspi_eoq_write(struct fsl_dspi *dspi) > */ > if (tx_word && (dspi->len == 1)) { > dspi->dataflags |= TRAN_STATE_WORD_ODD_NUM; > - regmap_update_bits(dspi->regmap, SPI_CTAR(0), > - SPI_FRAME_BITS_MASK, SPI_FRAME_BITS(8)); > + dspi_updatel(dspi, SPI_FRAME_BITS_MASK, > + SPI_FRAME_BITS(8), SPI_CTAR(0)); > tx_word = 0; > } > > @@ -309,7 +341,7 @@ static int dspi_eoq_write(struct fsl_dspi *dspi) > } else if (tx_word && (dspi->len == 1)) > dspi_pushr |= SPI_PUSHR_EOQ; > > - regmap_write(dspi->regmap, SPI_PUSHR, dspi_pushr); > + dspi_writel(dspi, dspi_pushr, SPI_PUSHR); > > tx_count++; > } > @@ -343,8 +375,8 @@ static int dspi_tcfq_write(struct fsl_dspi *dspi) > > if (tx_word && (dspi->len == 1)) { > dspi->dataflags |= TRAN_STATE_WORD_ODD_NUM; > - regmap_update_bits(dspi->regmap, SPI_CTAR(0), > - SPI_FRAME_BITS_MASK, SPI_FRAME_BITS(8)); > + dspi_updatel(dspi, SPI_FRAME_BITS_MASK, > + SPI_FRAME_BITS(8), SPI_CTAR(0)); > tx_word = 0; > } > > @@ -353,7 +385,7 @@ static int dspi_tcfq_write(struct fsl_dspi *dspi) > if ((dspi->cs_change) && (!dspi->len)) > dspi_pushr &= ~SPI_PUSHR_CONT; > > - regmap_write(dspi->regmap, SPI_PUSHR, dspi_pushr); > + dspi_writel(dspi, dspi_pushr, SPI_PUSHR); > > return tx_word + 1; > } > @@ -378,7 +410,7 @@ static int dspi_transfer_one_message(struct spi_master *master, > enum dspi_trans_mode trans_mode; > u32 spi_tcr; > > - regmap_read(dspi->regmap, SPI_TCR, &spi_tcr); > + spi_tcr = dspi_readl(dspi, SPI_TCR); > dspi->spi_tcnt = SPI_TCR_GET_TCNT(spi_tcr); > > message->actual_length = 0; > @@ -407,21 +439,19 @@ static int dspi_transfer_one_message(struct spi_master *master, > if (!dspi->tx) > dspi->dataflags |= TRAN_STATE_TX_VOID; > > - regmap_write(dspi->regmap, SPI_MCR, dspi->cur_chip->mcr_val); > - regmap_update_bits(dspi->regmap, SPI_MCR, > - SPI_MCR_CLR_TXF | SPI_MCR_CLR_RXF, > - SPI_MCR_CLR_TXF | SPI_MCR_CLR_RXF); > - regmap_write(dspi->regmap, SPI_CTAR(0), > - dspi->cur_chip->ctar_val); > + dspi_writel(dspi, dspi->cur_chip->mcr_val, SPI_MCR); > + dspi_updatel(dspi, SPI_MCR_CLR_TXF | SPI_MCR_CLR_RXF, > + SPI_MCR_CLR_TXF | SPI_MCR_CLR_RXF, SPI_MCR); > + dspi_writel(dspi, dspi->cur_chip->ctar_val, SPI_CTAR(0)); > > trans_mode = dspi->devtype_data->trans_mode; > switch (trans_mode) { > case DSPI_EOQ_MODE: > - regmap_write(dspi->regmap, SPI_RSER, SPI_RSER_EOQFE); > + dspi_writel(dspi, SPI_RSER_EOQFE, SPI_RSER); > dspi_eoq_write(dspi); > break; > case DSPI_TCFQ_MODE: > - regmap_write(dspi->regmap, SPI_RSER, SPI_RSER_TCFQE); > + dspi_writel(dspi, SPI_RSER_TCFQE, SPI_RSER); > dspi_tcfq_write(dspi); > break; > default: > @@ -525,14 +555,14 @@ static irqreturn_t dspi_interrupt(int irq, void *dev_id) > u32 spi_tcnt, tcnt_diff; > int tx_word; > > - regmap_read(dspi->regmap, SPI_SR, &spi_sr); > - regmap_write(dspi->regmap, SPI_SR, spi_sr); > + spi_sr = dspi_readl(dspi, SPI_SR); > + dspi_writel(dspi, spi_sr, SPI_SR); > > > if (spi_sr & (SPI_SR_EOQF | SPI_SR_TCFQF)) { > tx_word = is_double_byte_mode(dspi); > > - regmap_read(dspi->regmap, SPI_TCR, &spi_tcr); > + spi_tcr = dspi_readl(dspi, SPI_TCR); > spi_tcnt = SPI_TCR_GET_TCNT(spi_tcr); > /* > * The width of SPI Transfer Counter in SPI_TCR is 16bits, > @@ -569,10 +599,10 @@ static irqreturn_t dspi_interrupt(int irq, void *dev_id) > > if (!dspi->len) { > if (dspi->dataflags & TRAN_STATE_WORD_ODD_NUM) { > - regmap_update_bits(dspi->regmap, > - SPI_CTAR(0), > - SPI_FRAME_BITS_MASK, > - SPI_FRAME_BITS(16)); > + dspi_updatel(dspi, > + SPI_FRAME_BITS_MASK, > + SPI_FRAME_BITS(16), > + SPI_CTAR(0)); > dspi->dataflags &= ~TRAN_STATE_WORD_ODD_NUM; > } > > @@ -636,13 +666,6 @@ static int dspi_resume(struct device *dev) > > static SIMPLE_DEV_PM_OPS(dspi_pm, dspi_suspend, dspi_resume); > > -static const struct regmap_config dspi_regmap_config = { > - .reg_bits = 32, > - .val_bits = 32, > - .reg_stride = 4, > - .max_register = 0x88, > -}; > - > static int dspi_probe(struct platform_device *pdev) > { > struct device_node *np = pdev->dev.of_node; > @@ -700,13 +723,7 @@ static int dspi_probe(struct platform_device *pdev) > goto out_master_put; > } > > - dspi->regmap = devm_regmap_init_mmio_clk(&pdev->dev, NULL, base, > - &dspi_regmap_config); > - if (IS_ERR(dspi->regmap)) { > - dev_err(&pdev->dev, "failed to init regmap: %ld\n", > - PTR_ERR(dspi->regmap)); > - return PTR_ERR(dspi->regmap); > - } > + dspi->iobase = base; > > dspi->irq = platform_get_irq(pdev, 0); > if (dspi->irq < 0) { > @@ -730,6 +747,8 @@ static int dspi_probe(struct platform_device *pdev) > } > clk_prepare_enable(dspi->clk); > > + dspi->big_endian = of_property_read_bool(np, "big-endian"); > + > master->max_speed_hz = > clk_get_rate(dspi->clk) / dspi->devtype_data->max_clock_factor; > > -- > 2.1.0.27.g96db324 > > -- > To unsubscribe from this list: send the line "unsubscribe devicetree" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
T24gV2VkLCBNYXkgMDQsIDIwMTYgYXQgNTozMyBBTSwgcGt1Lmxlb0BnbWFpbC5jb20gd3JvdGU6 DQo+IE9uIFdlZCwgQXByIDI3LCAyMDE2IGF0IDM6MTIgQU0sIFl1YW4gWWFvIDx5YW8ueXVhbkBm cmVlc2NhbGUuY29tPiB3cm90ZToNCj4gPiBGcm9tOiBZdWFuIFlhbyA8eWFvLnl1YW5AbnhwLmNv bT4NCj4gPg0KPiA+IFRoZSBxU1BJIGNvbnRyb2xsZXIncyBlbmRpYW4gaXMgaW5kZXBlbmRlbnQg b2YgdGhlIENQVSBjb3JlJ3MgZW5kaWFuLg0KPiANCj4gRFNQST8NCg0KVGhhbmtzIGZvciB5b3Vy IHJldmlldy4NClNvcnJ5IGZvciB0aGUgdHlwbywgSSB3aWxsIHVwZGF0ZSBpbiB0aGUgbmV4dCB2 ZXJzaW9uLg0KIA0KPiA+DQo+ID4gRm9yIGVnLCBDb3JlIG9uIE5YUCBMUzEwNDNBIFNvQyBpcyBs aXR0bGUgZW5kaWFuIGJ1dCBEU1BJIGlzIGJpZw0KPiA+IGVuZGlhbiB3aGVyZWFzIENvcmUgb24g TFMyMDgwQSBTb0MgaXMgbGl0dGxlIGVuZGlhbiBhbmQgRFNQSSBhbHNvIGlzDQo+ID4gbGl0dGxl IGVuZGlhbiBvbiB0aGUgc2FtZSBjb3JlLg0KPiANCj4gSXQncyBub3QgY29ycmVjdCB0byBzYXkg dGhlIFNvQyBpcyBsaXR0bGUgZW5kaWFuIGZvciBMUzEwNDMgb3IgTFMyMDgwLg0KPiBOb3QgYWxs IGRldmljZXMgb24gdGhlIFNvQyBhcmUgc2hhcmluZyB0aGUgc2FtZSBlbmRpYW5uZXNzIGFuZCB0 aGUgY29yZQ0KPiBlbmRpYW5uZXNzIGlzIGNvbmZpZ3VyYWJsZS4NCg0KT2ssIEkgd2lsbCB1cGRh dGUgaXQuDQogDQoNCj4gDQo+ID4NCj4gPiBBdCBmaXJzdCB3ZSB1c2UgcmVnbWFwIHRvIGNvdmVy IHRoaXMgaXNzdWUuDQo+ID4gQnV0IHRoZSByZWdtYXAgaXMgZGVzaWduZWQgdG9vIGxhcmdlIGFu ZCBjb21wbGV4Lg0KPiA+IFRoZSBpc3N1ZSBmb3IgcmVnbWFwIG9mdGVuIGVmZmVjdCB0aGUgRFNQ SSdzIHN0YWJpbGl0eS4NCj4gDQo+IFRoaXMgaXMgbm90IHN0YWJpbGl0eSBpc3N1ZSBidXQgZnVu Y3Rpb25hbCBpc3N1ZS4NCg0KT2ssIEkgd2lsbCB1cGRhdGUgaXQuDQoNCj4gDQo+ID4NCj4gPiBE U1BJIGRyaXZlciBqdXN0IG9ubHkgbmVlZCBhIGVmZmVjdGl2ZSB3YXkgdG8gUi9XIHRoZSBjb250 cm9sbGVyDQo+ID4gcmVnaXN0ZXIgd2l0aCBCRSBvciBMRSBtb2RlLg0KPiA+DQo+ID4gU28gaXQn cyBiZXR0ZXIgdG8gcGFja2FnaW5nIGEgc2FtcGxlIGZ1bmN0aW9uLg0KPiA+IFRoaXMgd2lsbCBt YWtlIHRoZSBEU1BJIGRyaXZlciBtb3JlIHN0YWJpbGl0eSBhbmQgaGlnaCBlZmZlY3RpdmUuDQo+ ID4NCj4gPiBTaWduZWQtb2ZmLWJ5OiBZdWFuIFlhbyA8eWFvLnl1YW5AbnhwLmNvbT4NCj4gPiAt LS0NCj4gPiAgZHJpdmVycy9zcGkvc3BpLWZzbC1kc3BpLmMgfCA5OQ0KPiA+ICsrKysrKysrKysr KysrKysrKysrKysrKysrKy0tLS0tLS0tLS0tLS0tLS0tLS0NCj4gPiAgMSBmaWxlIGNoYW5nZWQs IDU5IGluc2VydGlvbnMoKyksIDQwIGRlbGV0aW9ucygtKQ0KPiA+DQo+ID4gZGlmZiAtLWdpdCBh L2RyaXZlcnMvc3BpL3NwaS1mc2wtZHNwaS5jIGIvZHJpdmVycy9zcGkvc3BpLWZzbC1kc3BpLmMN Cj4gPiBpbmRleCA5ZTlkYWRiLi45YTYxNTcyIDEwMDY0NA0KPiA+IC0tLSBhL2RyaXZlcnMvc3Bp L3NwaS1mc2wtZHNwaS5jDQo+ID4gKysrIGIvZHJpdmVycy9zcGkvc3BpLWZzbC1kc3BpLmMNCj4g PiBAQCAtMjcsNyArMjcsNiBAQA0KPiA+ICAjaW5jbHVkZSA8bGludXgvcGluY3RybC9jb25zdW1l ci5oPg0KPiA+ICAjaW5jbHVkZSA8bGludXgvcGxhdGZvcm1fZGV2aWNlLmg+DQo+ID4gICNpbmNs dWRlIDxsaW51eC9wbV9ydW50aW1lLmg+DQo+ID4gLSNpbmNsdWRlIDxsaW51eC9yZWdtYXAuaD4N Cj4gPiAgI2luY2x1ZGUgPGxpbnV4L3NjaGVkLmg+DQo+ID4gICNpbmNsdWRlIDxsaW51eC9zcGkv c3BpLmg+DQo+ID4gICNpbmNsdWRlIDxsaW51eC9zcGkvc3BpX2JpdGJhbmcuaD4NCj4gPiBAQCAt MTQzLDcgKzE0Miw3IEBAIHN0cnVjdCBmc2xfZHNwaSB7DQo+ID4gICAgICAgICBzdHJ1Y3Qgc3Bp X21hc3RlciAgICAgICAqbWFzdGVyOw0KPiA+ICAgICAgICAgc3RydWN0IHBsYXRmb3JtX2Rldmlj ZSAgKnBkZXY7DQo+ID4NCj4gPiAtICAgICAgIHN0cnVjdCByZWdtYXAgICAgICAgICAgICpyZWdt YXA7DQo+ID4gKyAgICAgICB2b2lkIF9faW9tZW0gICAgICAgICAgICAqaW9iYXNlOw0KPiA+ICAg ICAgICAgaW50ICAgICAgICAgICAgICAgICAgICAgaXJxOw0KPiA+ICAgICAgICAgc3RydWN0IGNs ayAgICAgICAgICAgICAgKmNsazsNCj4gPg0KPiA+IEBAIC0xNjUsMTMgKzE2NCw0NiBAQCBzdHJ1 Y3QgZnNsX2RzcGkgew0KPiA+ICAgICAgICAgdTMyICAgICAgICAgICAgICAgICAgICAgd2FpdGZs YWdzOw0KPiA+DQo+ID4gICAgICAgICB1MzIgICAgICAgICAgICAgICAgICAgICBzcGlfdGNudDsN Cj4gPiArICAgICAgIGJvb2wgICAgICAgICAgICAgICAgICAgIGJpZ19lbmRpYW47DQo+ID4gIH07 DQo+ID4NCj4gPiArLyoNCj4gPiArICogUi9XIGZ1bmN0aW9ucyBmb3IgYmlnLSBvciBsaXR0bGUt ZW5kaWFuIHJlZ2lzdGVyczoNCj4gPiArICogVGhlIHFTUEkgY29udHJvbGxlcidzIGVuZGlhbiBp cyBpbmRlcGVuZGVudCBvZiB0aGUgQ1BVIGNvcmUncyBlbmRpYW4uDQo+IA0KPiBEU1BJPw0KDQpZ ZXMsIERTUEksIEkgd2lsbCB1cGRhdGUgaXQuDQo+IA0KPiA+ICsgKi8NCj4gPiArc3RhdGljIHZv aWQgZHNwaV93cml0ZWwoc3RydWN0IGZzbF9kc3BpICpkLCB1MzIgdmFsLCB1MzIgb2Zmc2V0KSB7 DQo+ID4gKyAgICAgICBpZiAoZC0+YmlnX2VuZGlhbikNCj4gPiArICAgICAgICAgICAgICAgaW93 cml0ZTMyYmUodmFsLCBkLT5pb2Jhc2UgKyBvZmZzZXQpOw0KPiA+ICsgICAgICAgZWxzZQ0KPiA+ ICsgICAgICAgICAgICAgICBpb3dyaXRlMzIodmFsLCBkLT5pb2Jhc2UgKyBvZmZzZXQpOyB9DQo+ ID4gKw0KPiA+ICtzdGF0aWMgdTMyIGRzcGlfcmVhZGwoc3RydWN0IGZzbF9kc3BpICpkLCB1MzIg b2Zmc2V0KSB7DQo+ID4gKyAgICAgICBpZiAoZC0+YmlnX2VuZGlhbikNCj4gPiArICAgICAgICAg ICAgICAgcmV0dXJuIGlvcmVhZDMyYmUoZC0+aW9iYXNlICsgb2Zmc2V0KTsNCj4gPiArICAgICAg IGVsc2UNCj4gPiArICAgICAgICAgICAgICAgcmV0dXJuIGlvcmVhZDMyKGQtPmlvYmFzZSArIG9m ZnNldCk7IH0NCj4gPiArDQo+ID4gK3N0YXRpYyB2b2lkIGRzcGlfdXBkYXRlbChzdHJ1Y3QgZnNs X2RzcGkgKmQsIHUzMiBtYXNrLCB1MzIgdmFsLCB1MzINCj4gPiArb2Zmc2V0KSB7DQo+ID4gKyAg ICAgICB1MzIgdG1wLCBvcmlnOw0KPiA+ICsNCj4gPiArICAgICAgIG9yaWcgPSBkc3BpX3JlYWRs KGQsIG9mZnNldCk7DQo+ID4gKw0KPiA+ICsgICAgICAgdG1wID0gb3JpZyAmIH5tYXNrOw0KPiA+ ICsgICAgICAgdG1wIHw9IHZhbCAmIG1hc2s7DQo+ID4gKw0KPiA+ICsgICAgICAgZHNwaV93cml0 ZWwoZCwgdG1wLCBvZmZzZXQpOw0KPiA+ICt9DQo+ID4gKw0KPiA+ICBzdGF0aWMgaW5saW5lIGlu dCBpc19kb3VibGVfYnl0ZV9tb2RlKHN0cnVjdCBmc2xfZHNwaSAqZHNwaSkgIHsNCj4gPiAgICAg ICAgIHVuc2lnbmVkIGludCB2YWw7DQo+ID4NCj4gPiAtICAgICAgIHJlZ21hcF9yZWFkKGRzcGkt PnJlZ21hcCwgU1BJX0NUQVIoMCksICZ2YWwpOw0KPiA+ICsgICAgICAgdmFsID0gZHNwaV9yZWFk bChkc3BpLCBTUElfQ1RBUigwKSk7DQo+ID4NCj4gPiAgICAgICAgIHJldHVybiAoKHZhbCAmIFNQ SV9GUkFNRV9CSVRTX01BU0spID09IFNQSV9GUkFNRV9CSVRTKDgpKSA/IDANCj4gPiA6IDE7ICB9 IEBAIC0yNzAsNyArMzAyLDcgQEAgc3RhdGljIHZvaWQgZHNwaV9kYXRhX2Zyb21fcG9wcihzdHJ1 Y3QNCj4gPiBmc2xfZHNwaSAqZHNwaSwgaW50IHJ4X3dvcmQpDQo+ID4gICAgICAgICB1MTYgZDsN Cj4gPiAgICAgICAgIHVuc2lnbmVkIGludCB2YWw7DQo+ID4NCj4gPiAtICAgICAgIHJlZ21hcF9y ZWFkKGRzcGktPnJlZ21hcCwgU1BJX1BPUFIsICZ2YWwpOw0KPiA+ICsgICAgICAgdmFsID0gZHNw aV9yZWFkbChkc3BpLCBTUElfUE9QUik7DQo+ID4gICAgICAgICBkID0gU1BJX1BPUFJfUlhEQVRB KHZhbCk7DQo+ID4NCj4gPiAgICAgICAgIGlmICghKGRzcGktPmRhdGFmbGFncyAmIFRSQU5fU1RB VEVfUlhfVk9JRCkpIEBAIC0yOTQsOCArMzI2LDgNCj4gPiBAQCBzdGF0aWMgaW50IGRzcGlfZW9x X3dyaXRlKHN0cnVjdCBmc2xfZHNwaSAqZHNwaSkNCj4gPiAgICAgICAgICAgICAgICAgICovDQo+ ID4gICAgICAgICAgICAgICAgIGlmICh0eF93b3JkICYmIChkc3BpLT5sZW4gPT0gMSkpIHsNCj4g PiAgICAgICAgICAgICAgICAgICAgICAgICBkc3BpLT5kYXRhZmxhZ3MgfD0gVFJBTl9TVEFURV9X T1JEX09ERF9OVU07DQo+ID4gLSAgICAgICAgICAgICAgICAgICAgICAgcmVnbWFwX3VwZGF0ZV9i aXRzKGRzcGktPnJlZ21hcCwgU1BJX0NUQVIoMCksDQo+ID4gLSAgICAgICAgICAgICAgICAgICAg ICAgICAgICAgICAgICAgICAgIFNQSV9GUkFNRV9CSVRTX01BU0ssIFNQSV9GUkFNRV9CSVRTKDgp KTsNCj4gPiArICAgICAgICAgICAgICAgICAgICAgICBkc3BpX3VwZGF0ZWwoZHNwaSwgU1BJX0ZS QU1FX0JJVFNfTUFTSywNCj4gPiArICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg ICAgU1BJX0ZSQU1FX0JJVFMoOCksDQo+ID4gKyBTUElfQ1RBUigwKSk7DQo+ID4gICAgICAgICAg ICAgICAgICAgICAgICAgdHhfd29yZCA9IDA7DQo+ID4gICAgICAgICAgICAgICAgIH0NCj4gPg0K PiA+IEBAIC0zMDksNyArMzQxLDcgQEAgc3RhdGljIGludCBkc3BpX2VvcV93cml0ZShzdHJ1Y3Qg ZnNsX2RzcGkgKmRzcGkpDQo+ID4gICAgICAgICAgICAgICAgIH0gZWxzZSBpZiAodHhfd29yZCAm JiAoZHNwaS0+bGVuID09IDEpKQ0KPiA+ICAgICAgICAgICAgICAgICAgICAgICAgIGRzcGlfcHVz aHIgfD0gU1BJX1BVU0hSX0VPUTsNCj4gPg0KPiA+IC0gICAgICAgICAgICAgICByZWdtYXBfd3Jp dGUoZHNwaS0+cmVnbWFwLCBTUElfUFVTSFIsIGRzcGlfcHVzaHIpOw0KPiA+ICsgICAgICAgICAg ICAgICBkc3BpX3dyaXRlbChkc3BpLCBkc3BpX3B1c2hyLCBTUElfUFVTSFIpOw0KPiA+DQo+ID4g ICAgICAgICAgICAgICAgIHR4X2NvdW50Kys7DQo+ID4gICAgICAgICB9DQo+ID4gQEAgLTM0Myw4 ICszNzUsOCBAQCBzdGF0aWMgaW50IGRzcGlfdGNmcV93cml0ZShzdHJ1Y3QgZnNsX2RzcGkgKmRz cGkpDQo+ID4NCj4gPiAgICAgICAgIGlmICh0eF93b3JkICYmIChkc3BpLT5sZW4gPT0gMSkpIHsN Cj4gPiAgICAgICAgICAgICAgICAgZHNwaS0+ZGF0YWZsYWdzIHw9IFRSQU5fU1RBVEVfV09SRF9P RERfTlVNOw0KPiA+IC0gICAgICAgICAgICAgICByZWdtYXBfdXBkYXRlX2JpdHMoZHNwaS0+cmVn bWFwLCBTUElfQ1RBUigwKSwNCj4gPiAtICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIFNQ SV9GUkFNRV9CSVRTX01BU0ssIFNQSV9GUkFNRV9CSVRTKDgpKTsNCj4gPiArICAgICAgICAgICAg ICAgZHNwaV91cGRhdGVsKGRzcGksIFNQSV9GUkFNRV9CSVRTX01BU0ssDQo+ID4gKyAgICAgICAg ICAgICAgICAgICAgICAgICAgICAgICBTUElfRlJBTUVfQklUUyg4KSwgU1BJX0NUQVIoMCkpOw0K PiA+ICAgICAgICAgICAgICAgICB0eF93b3JkID0gMDsNCj4gPiAgICAgICAgIH0NCj4gPg0KPiA+ IEBAIC0zNTMsNyArMzg1LDcgQEAgc3RhdGljIGludCBkc3BpX3RjZnFfd3JpdGUoc3RydWN0IGZz bF9kc3BpICpkc3BpKQ0KPiA+ICAgICAgICAgaWYgKChkc3BpLT5jc19jaGFuZ2UpICYmICghZHNw aS0+bGVuKSkNCj4gPiAgICAgICAgICAgICAgICAgZHNwaV9wdXNociAmPSB+U1BJX1BVU0hSX0NP TlQ7DQo+ID4NCj4gPiAtICAgICAgIHJlZ21hcF93cml0ZShkc3BpLT5yZWdtYXAsIFNQSV9QVVNI UiwgZHNwaV9wdXNocik7DQo+ID4gKyAgICAgICBkc3BpX3dyaXRlbChkc3BpLCBkc3BpX3B1c2hy LCBTUElfUFVTSFIpOw0KPiA+DQo+ID4gICAgICAgICByZXR1cm4gdHhfd29yZCArIDE7DQo+ID4g IH0NCj4gPiBAQCAtMzc4LDcgKzQxMCw3IEBAIHN0YXRpYyBpbnQgZHNwaV90cmFuc2Zlcl9vbmVf bWVzc2FnZShzdHJ1Y3QNCj4gc3BpX21hc3RlciAqbWFzdGVyLA0KPiA+ICAgICAgICAgZW51bSBk c3BpX3RyYW5zX21vZGUgdHJhbnNfbW9kZTsNCj4gPiAgICAgICAgIHUzMiBzcGlfdGNyOw0KPiA+ DQo+ID4gLSAgICAgICByZWdtYXBfcmVhZChkc3BpLT5yZWdtYXAsIFNQSV9UQ1IsICZzcGlfdGNy KTsNCj4gPiArICAgICAgIHNwaV90Y3IgPSBkc3BpX3JlYWRsKGRzcGksIFNQSV9UQ1IpOw0KPiA+ ICAgICAgICAgZHNwaS0+c3BpX3RjbnQgPSBTUElfVENSX0dFVF9UQ05UKHNwaV90Y3IpOw0KPiA+ DQo+ID4gICAgICAgICBtZXNzYWdlLT5hY3R1YWxfbGVuZ3RoID0gMDsNCj4gPiBAQCAtNDA3LDIx ICs0MzksMTkgQEAgc3RhdGljIGludCBkc3BpX3RyYW5zZmVyX29uZV9tZXNzYWdlKHN0cnVjdA0K PiBzcGlfbWFzdGVyICptYXN0ZXIsDQo+ID4gICAgICAgICAgICAgICAgIGlmICghZHNwaS0+dHgp DQo+ID4gICAgICAgICAgICAgICAgICAgICAgICAgZHNwaS0+ZGF0YWZsYWdzIHw9IFRSQU5fU1RB VEVfVFhfVk9JRDsNCj4gPg0KPiA+IC0gICAgICAgICAgICAgICByZWdtYXBfd3JpdGUoZHNwaS0+ cmVnbWFwLCBTUElfTUNSLCBkc3BpLT5jdXJfY2hpcC0+bWNyX3ZhbCk7DQo+ID4gLSAgICAgICAg ICAgICAgIHJlZ21hcF91cGRhdGVfYml0cyhkc3BpLT5yZWdtYXAsIFNQSV9NQ1IsDQo+ID4gLSAg ICAgICAgICAgICAgICAgICAgICAgICAgICAgICBTUElfTUNSX0NMUl9UWEYgfCBTUElfTUNSX0NM Ul9SWEYsDQo+ID4gLSAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICBTUElfTUNSX0NMUl9U WEYgfCBTUElfTUNSX0NMUl9SWEYpOw0KPiA+IC0gICAgICAgICAgICAgICByZWdtYXBfd3JpdGUo ZHNwaS0+cmVnbWFwLCBTUElfQ1RBUigwKSwNCj4gPiAtICAgICAgICAgICAgICAgICAgICAgICAg ICAgICAgIGRzcGktPmN1cl9jaGlwLT5jdGFyX3ZhbCk7DQo+ID4gKyAgICAgICAgICAgICAgIGRz cGlfd3JpdGVsKGRzcGksIGRzcGktPmN1cl9jaGlwLT5tY3JfdmFsLCBTUElfTUNSKTsNCj4gPiAr ICAgICAgICAgICAgICAgZHNwaV91cGRhdGVsKGRzcGksIFNQSV9NQ1JfQ0xSX1RYRiB8IFNQSV9N Q1JfQ0xSX1JYRiwNCj4gPiArICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIFNQSV9N Q1JfQ0xSX1RYRiB8IFNQSV9NQ1JfQ0xSX1JYRiwgU1BJX01DUik7DQo+ID4gKyAgICAgICAgICAg ICAgIGRzcGlfd3JpdGVsKGRzcGksIGRzcGktPmN1cl9jaGlwLT5jdGFyX3ZhbCwNCj4gPiArIFNQ SV9DVEFSKDApKTsNCj4gPg0KPiA+ICAgICAgICAgICAgICAgICB0cmFuc19tb2RlID0gZHNwaS0+ ZGV2dHlwZV9kYXRhLT50cmFuc19tb2RlOw0KPiA+ICAgICAgICAgICAgICAgICBzd2l0Y2ggKHRy YW5zX21vZGUpIHsNCj4gPiAgICAgICAgICAgICAgICAgY2FzZSBEU1BJX0VPUV9NT0RFOg0KPiA+ IC0gICAgICAgICAgICAgICAgICAgICAgIHJlZ21hcF93cml0ZShkc3BpLT5yZWdtYXAsIFNQSV9S U0VSLCBTUElfUlNFUl9FT1FGRSk7DQo+ID4gKyAgICAgICAgICAgICAgICAgICAgICAgZHNwaV93 cml0ZWwoZHNwaSwgU1BJX1JTRVJfRU9RRkUsIFNQSV9SU0VSKTsNCj4gPiAgICAgICAgICAgICAg ICAgICAgICAgICBkc3BpX2VvcV93cml0ZShkc3BpKTsNCj4gPiAgICAgICAgICAgICAgICAgICAg ICAgICBicmVhazsNCj4gPiAgICAgICAgICAgICAgICAgY2FzZSBEU1BJX1RDRlFfTU9ERToNCj4g PiAtICAgICAgICAgICAgICAgICAgICAgICByZWdtYXBfd3JpdGUoZHNwaS0+cmVnbWFwLCBTUElf UlNFUiwgU1BJX1JTRVJfVENGUUUpOw0KPiA+ICsgICAgICAgICAgICAgICAgICAgICAgIGRzcGlf d3JpdGVsKGRzcGksIFNQSV9SU0VSX1RDRlFFLCBTUElfUlNFUik7DQo+ID4gICAgICAgICAgICAg ICAgICAgICAgICAgZHNwaV90Y2ZxX3dyaXRlKGRzcGkpOw0KPiA+ICAgICAgICAgICAgICAgICAg ICAgICAgIGJyZWFrOw0KPiA+ICAgICAgICAgICAgICAgICBkZWZhdWx0Og0KPiA+IEBAIC01MjUs MTQgKzU1NSwxNCBAQCBzdGF0aWMgaXJxcmV0dXJuX3QgZHNwaV9pbnRlcnJ1cHQoaW50IGlycSwg dm9pZA0KPiAqZGV2X2lkKQ0KPiA+ICAgICAgICAgdTMyIHNwaV90Y250LCB0Y250X2RpZmY7DQo+ ID4gICAgICAgICBpbnQgdHhfd29yZDsNCj4gPg0KPiA+IC0gICAgICAgcmVnbWFwX3JlYWQoZHNw aS0+cmVnbWFwLCBTUElfU1IsICZzcGlfc3IpOw0KPiA+IC0gICAgICAgcmVnbWFwX3dyaXRlKGRz cGktPnJlZ21hcCwgU1BJX1NSLCBzcGlfc3IpOw0KPiA+ICsgICAgICAgc3BpX3NyID0gZHNwaV9y ZWFkbChkc3BpLCBTUElfU1IpOw0KPiA+ICsgICAgICAgZHNwaV93cml0ZWwoZHNwaSwgc3BpX3Ny LCBTUElfU1IpOw0KPiA+DQo+ID4NCj4gPiAgICAgICAgIGlmIChzcGlfc3IgJiAoU1BJX1NSX0VP UUYgfCBTUElfU1JfVENGUUYpKSB7DQo+ID4gICAgICAgICAgICAgICAgIHR4X3dvcmQgPSBpc19k b3VibGVfYnl0ZV9tb2RlKGRzcGkpOw0KPiA+DQo+ID4gLSAgICAgICAgICAgICAgIHJlZ21hcF9y ZWFkKGRzcGktPnJlZ21hcCwgU1BJX1RDUiwgJnNwaV90Y3IpOw0KPiA+ICsgICAgICAgICAgICAg ICBzcGlfdGNyID0gZHNwaV9yZWFkbChkc3BpLCBTUElfVENSKTsNCj4gPiAgICAgICAgICAgICAg ICAgc3BpX3RjbnQgPSBTUElfVENSX0dFVF9UQ05UKHNwaV90Y3IpOw0KPiA+ICAgICAgICAgICAg ICAgICAvKg0KPiA+ICAgICAgICAgICAgICAgICAgKiBUaGUgd2lkdGggb2YgU1BJIFRyYW5zZmVy IENvdW50ZXIgaW4gU1BJX1RDUiBpcw0KPiA+IDE2Yml0cywgQEAgLTU2OSwxMCArNTk5LDEwIEBA IHN0YXRpYyBpcnFyZXR1cm5fdCBkc3BpX2ludGVycnVwdChpbnQNCj4gPiBpcnEsIHZvaWQgKmRl dl9pZCkNCj4gPg0KPiA+ICAgICAgICAgICAgICAgICBpZiAoIWRzcGktPmxlbikgew0KPiA+ICAg ICAgICAgICAgICAgICAgICAgICAgIGlmIChkc3BpLT5kYXRhZmxhZ3MgJiBUUkFOX1NUQVRFX1dP UkRfT0REX05VTSkgew0KPiA+IC0gICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgcmVnbWFw X3VwZGF0ZV9iaXRzKGRzcGktPnJlZ21hcCwNCj4gPiAtICAgICAgICAgICAgICAgICAgICAgICAg ICAgICAgICAgICAgICAgICAgICAgICAgICBTUElfQ1RBUigwKSwNCj4gPiAtICAgICAgICAgICAg ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICBTUElfRlJBTUVfQklUU19NQVNL LA0KPiA+IC0gICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg IFNQSV9GUkFNRV9CSVRTKDE2KSk7DQo+ID4gKyAgICAgICAgICAgICAgICAgICAgICAgICAgICAg ICBkc3BpX3VwZGF0ZWwoZHNwaSwNCj4gPiArICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg ICAgICAgICAgICAgICBTUElfRlJBTUVfQklUU19NQVNLLA0KPiA+ICsgICAgICAgICAgICAgICAg ICAgICAgICAgICAgICAgICAgICAgICAgICAgIFNQSV9GUkFNRV9CSVRTKDE2KSwNCj4gPiArICAg ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICBTUElfQ1RBUigwKSk7DQo+ ID4gICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICBkc3BpLT5kYXRhZmxhZ3MgJj0gflRS QU5fU1RBVEVfV09SRF9PRERfTlVNOw0KPiA+ICAgICAgICAgICAgICAgICAgICAgICAgIH0NCj4g Pg0KPiA+IEBAIC02MzYsMTMgKzY2Niw2IEBAIHN0YXRpYyBpbnQgZHNwaV9yZXN1bWUoc3RydWN0 IGRldmljZSAqZGV2KQ0KPiA+DQo+ID4gIHN0YXRpYyBTSU1QTEVfREVWX1BNX09QUyhkc3BpX3Bt LCBkc3BpX3N1c3BlbmQsIGRzcGlfcmVzdW1lKTsNCj4gPg0KPiA+IC1zdGF0aWMgY29uc3Qgc3Ry dWN0IHJlZ21hcF9jb25maWcgZHNwaV9yZWdtYXBfY29uZmlnID0gew0KPiA+IC0gICAgICAgLnJl Z19iaXRzID0gMzIsDQo+ID4gLSAgICAgICAudmFsX2JpdHMgPSAzMiwNCj4gPiAtICAgICAgIC5y ZWdfc3RyaWRlID0gNCwNCj4gPiAtICAgICAgIC5tYXhfcmVnaXN0ZXIgPSAweDg4LA0KPiA+IC19 Ow0KPiA+IC0NCj4gPiAgc3RhdGljIGludCBkc3BpX3Byb2JlKHN0cnVjdCBwbGF0Zm9ybV9kZXZp Y2UgKnBkZXYpICB7DQo+ID4gICAgICAgICBzdHJ1Y3QgZGV2aWNlX25vZGUgKm5wID0gcGRldi0+ ZGV2Lm9mX25vZGU7IEBAIC03MDAsMTMgKzcyMyw3DQo+ID4gQEAgc3RhdGljIGludCBkc3BpX3By b2JlKHN0cnVjdCBwbGF0Zm9ybV9kZXZpY2UgKnBkZXYpDQo+ID4gICAgICAgICAgICAgICAgIGdv dG8gb3V0X21hc3Rlcl9wdXQ7DQo+ID4gICAgICAgICB9DQo+ID4NCj4gPiAtICAgICAgIGRzcGkt PnJlZ21hcCA9IGRldm1fcmVnbWFwX2luaXRfbW1pb19jbGsoJnBkZXYtPmRldiwgTlVMTCwgYmFz ZSwNCj4gPiAtICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAm ZHNwaV9yZWdtYXBfY29uZmlnKTsNCj4gPiAtICAgICAgIGlmIChJU19FUlIoZHNwaS0+cmVnbWFw KSkgew0KPiA+IC0gICAgICAgICAgICAgICBkZXZfZXJyKCZwZGV2LT5kZXYsICJmYWlsZWQgdG8g aW5pdCByZWdtYXA6ICVsZFxuIiwNCj4gPiAtICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg IFBUUl9FUlIoZHNwaS0+cmVnbWFwKSk7DQo+ID4gLSAgICAgICAgICAgICAgIHJldHVybiBQVFJf RVJSKGRzcGktPnJlZ21hcCk7DQo+ID4gLSAgICAgICB9DQo+ID4gKyAgICAgICBkc3BpLT5pb2Jh c2UgPSBiYXNlOw0KPiA+DQo+ID4gICAgICAgICBkc3BpLT5pcnEgPSBwbGF0Zm9ybV9nZXRfaXJx KHBkZXYsIDApOw0KPiA+ICAgICAgICAgaWYgKGRzcGktPmlycSA8IDApIHsNCj4gPiBAQCAtNzMw LDYgKzc0Nyw4IEBAIHN0YXRpYyBpbnQgZHNwaV9wcm9iZShzdHJ1Y3QgcGxhdGZvcm1fZGV2aWNl ICpwZGV2KQ0KPiA+ICAgICAgICAgfQ0KPiA+ICAgICAgICAgY2xrX3ByZXBhcmVfZW5hYmxlKGRz cGktPmNsayk7DQo+ID4NCj4gPiArICAgICAgIGRzcGktPmJpZ19lbmRpYW4gPSBvZl9wcm9wZXJ0 eV9yZWFkX2Jvb2wobnAsICJiaWctZW5kaWFuIik7DQo+ID4gKw0KPiA+ICAgICAgICAgbWFzdGVy LT5tYXhfc3BlZWRfaHogPQ0KPiA+ICAgICAgICAgICAgICAgICBjbGtfZ2V0X3JhdGUoZHNwaS0+ Y2xrKSAvDQo+ID4gZHNwaS0+ZGV2dHlwZV9kYXRhLT5tYXhfY2xvY2tfZmFjdG9yOw0KPiA+DQo+ ID4gLS0NCj4gPiAyLjEuMC4yNy5nOTZkYjMyNA0KPiA+DQo+ID4gLS0NCj4gPiBUbyB1bnN1YnNj cmliZSBmcm9tIHRoaXMgbGlzdDogc2VuZCB0aGUgbGluZSAidW5zdWJzY3JpYmUgZGV2aWNldHJl ZSINCj4gPiBpbiB0aGUgYm9keSBvZiBhIG1lc3NhZ2UgdG8gbWFqb3Jkb21vQHZnZXIua2VybmVs Lm9yZyBNb3JlIG1ham9yZG9tbw0KPiA+IGluZm8gYXQgIGh0dHA6Ly92Z2VyLmtlcm5lbC5vcmcv bWFqb3Jkb21vLWluZm8uaHRtbA0K -- To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Mark Brown, There are two problems we have found for use regmap up to now. 1, Regmap will read device value depends on endian in .dtsi(DSPI in .dtsi file is big-endian) and readl()(readl() in arm64 is le32_to_cpu read) in system. That is the value read out is fixed regardless the kernel were compiled with CONFIG_CPU_BIG_ENDIAN yes or no. That makes the value(read from DSPI)can't be recognized in one endian mode(CONFIG_CPU_BIG_ENDIAN be set or not). The result is: DSPI can't working with CONFIG_CPU_BIG_ENDIAN set. 2, Sometimes regmap will generate some issues. Some regmap issues will cause the DSPI do not working. At 2016.4 we find the DSPI was not work, then we found that the regmap R/W the controller register sometimes not right. The result is: DSPI can't working with REGMAP API. Although the regmap issues was fast fixed by some giving person. But I'm still tend to use the reliable way. Regmap is nice and Maybe I should try to fix regmap instead of replacing regmap in my driver. But regmap is really very large and complex for DSPI driver. I think maybe the suitable way is the best way even the regmap is so good as everyone says. So I strongly suggest to use the dspi_writel / dspi_readl to instead of the regmap in DSPI driver. Best Regards, Yuan Yao > -----Original Message----- > From: Mark Brown [mailto:broonie@kernel.org] > Sent: Wednesday, April 27, 2016 11:26 PM > To: Yuan Yao <yao.yuan@freescale.com> > Cc: robh+dt@kernel.org; pawel.moll@arm.com; shawnguo@kernel.org; linux- > devel@gforge.freescale.net; linux-spi@vger.kernel.org; > devicetree@vger.kernel.org; Yao Yuan <yao.yuan@nxp.com>; Yang-Leo Li > <leoyang.li@nxp.com>; Scott Wood <scott.wood@nxp.com> > Subject: Re: [PATCH 1/2] spi: spi-fsl-dspi: replace regmap R/W with internal > implementation > > On Wed, Apr 27, 2016 at 04:12:35PM +0800, Yuan Yao wrote: > > > At first we use regmap to cover this issue. > > But the regmap is designed too large and complex. > > The issue for regmap often effect the DSPI's stability. > > I'm not convinced that open coding things is going to make life much easier > here. If there's problems in the underlying code then fixing those seems like a > more generally useful approach to things. What are the actual problems this is > intended to fix? -- To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, May 09, 2016 at 08:48:59AM +0000, Yao Yuan wrote: > Hi Mark Brown, Please don't top post, reply in line with needed context. This allows readers to readily follow the flow of conversation and understand what you are talking about and also helps ensure that everything in the discussion is being addressed. Please fix your mail client to word wrap within paragraphs at something substantially less than 80 columns. Doing this makes your messages much easier to read and reply to. > There are two problems we have found for use regmap up to now. > 1, Regmap will read device value depends on endian in .dtsi(DSPI in .dtsi file is big-endian) and readl()(readl() in arm64 is le32_to_cpu read) in system. > That is the value read out is fixed regardless the kernel were compiled with CONFIG_CPU_BIG_ENDIAN yes or no. That makes the value(read from DSPI)can't be recognized in one endian mode(CONFIG_CPU_BIG_ENDIAN be set or not). > The result is: DSPI can't working with CONFIG_CPU_BIG_ENDIAN set. This doesn't make sense. If the endianness of the device is specified in DT then the endianness of the CPU won't make a difference. > 2, Sometimes regmap will generate some issues. Some regmap issues will cause the DSPI do not working. > At 2016.4 we find the DSPI was not work, then we found that the regmap R/W the controller register sometimes not right. > The result is: DSPI can't working with REGMAP API. > Although the regmap issues was fast fixed by some giving person. But I'm still tend to use the reliable way. > Regmap is nice and Maybe I should try to fix regmap instead of replacing regmap in my driver. > But regmap is really very large and complex for DSPI driver. Yes, if you find bugs in generic frameworks you should fix them rather than just open coding something.
On Mon, May 09, 2016 at 06:29:59AM +0800, Mark Brown wrote: > On Mon, May 09, 2016 at 08:48:59AM +0000, Yao Yuan wrote: > > Hi Mark Brown, > > Please don't top post, reply in line with needed context. This allows readers to > readily follow the flow of conversation and understand what you are talking > about and also helps ensure that everything in the discussion is being > addressed. > > Please fix your mail client to word wrap within paragraphs at something > substantially less than 80 columns. Doing this makes your messages much > easier to read and reply to. > Ok, Thanks. > > There are two problems we have found for use regmap up to now. > > > 1, Regmap will read device value depends on endian in .dtsi(DSPI in .dtsi file is > big-endian) and readl()(readl() in arm64 is le32_to_cpu read) in system. > > That is the value read out is fixed regardless the kernel were compiled with > CONFIG_CPU_BIG_ENDIAN yes or no. That makes the value(read from > DSPI)can't be recognized in one endian mode(CONFIG_CPU_BIG_ENDIAN be > set or not). > > The result is: DSPI can't working with CONFIG_CPU_BIG_ENDIAN set. > > This doesn't make sense. If the endianness of the device is specified in DT then > the endianness of the CPU won't make a difference. > For example, if DSPI controller register is BE, so I set big-endian in DT. That means we should R/W the DSPI controller register with big-endian. Then I can think all of the value we R/W to/from DSPI controller register, we can Think they are the BE. So that we can understand it easy. That means no matter core is LE or BE, we both need get the value from DSPI register with big endian. But from regmap is not. When the core is little endian, I can get the big endian value form register like:0xaabbccdd. Then we can analysis and process this value. But once config the core to big endian, I get the value like:0xddccbbaa. It's the little endian. That's terrible for my driver to understand the value. > > 2, Sometimes regmap will generate some issues. Some regmap issues will > cause the DSPI do not working. > > At 2016.4 we find the DSPI was not work, then we found that the regmap > R/W the controller register sometimes not right. > > The result is: DSPI can't working with REGMAP API. > > Although the regmap issues was fast fixed by some giving person. But I'm still > tend to use the reliable way. > > > Regmap is nice and Maybe I should try to fix regmap instead of replacing > regmap in my driver. > > But regmap is really very large and complex for DSPI driver. > > Yes, if you find bugs in generic frameworks you should fix them rather than just > open coding something. -- To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, May 9, 2016 at 5:58 AM, Yao Yuan <yao.yuan@nxp.com> wrote: > On Mon, May 09, 2016 at 06:29:59AM +0800, Mark Brown wrote: >> On Mon, May 09, 2016 at 08:48:59AM +0000, Yao Yuan wrote: >> > Hi Mark Brown, >> >> Please don't top post, reply in line with needed context. This allows readers to >> readily follow the flow of conversation and understand what you are talking >> about and also helps ensure that everything in the discussion is being >> addressed. >> >> Please fix your mail client to word wrap within paragraphs at something >> substantially less than 80 columns. Doing this makes your messages much >> easier to read and reply to. >> > > Ok, Thanks. > >> > There are two problems we have found for use regmap up to now. >> >> > 1, Regmap will read device value depends on endian in .dtsi(DSPI in .dtsi file is >> big-endian) and readl()(readl() in arm64 is le32_to_cpu read) in system. >> > That is the value read out is fixed regardless the kernel were compiled with >> CONFIG_CPU_BIG_ENDIAN yes or no. That makes the value(read from >> DSPI)can't be recognized in one endian mode(CONFIG_CPU_BIG_ENDIAN be >> set or not). >> > The result is: DSPI can't working with CONFIG_CPU_BIG_ENDIAN set. >> >> This doesn't make sense. If the endianness of the device is specified in DT then >> the endianness of the CPU won't make a difference. >> > > For example, if DSPI controller register is BE, so I set big-endian in DT. > That means we should R/W the DSPI controller register with big-endian. > Then I can think all of the value we R/W to/from DSPI controller register, we can > Think they are the BE. > So that we can understand it easy. > > That means no matter core is LE or BE, we both need get the value from DSPI register > with big endian. > > But from regmap is not. > When the core is little endian, I can get the big endian value form register like:0xaabbccdd. > Then we can analysis and process this value. > But once config the core to big endian, I get the value like:0xddccbbaa. It's the little endian. > That's terrible for my driver to understand the value. This is just the result. Are you able to dig deeper to explain why this is happening? I checked the LS1043 reference manual, the register definition looks like little endian instead of the big endian defined in the LS1043 device tree. Can you confirm if the block is actually having big endian registers or little endian registers. Regards, Leo -- To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> -----Original Message----- > From: linux-devel-bounces@gforge.freescale.net [mailto:linux-devel- > bounces@gforge.freescale.net] On Behalf Of Leo Li > Sent: Thursday, May 12, 2016 6:20 AM > To: Yao Yuan > Cc: devicetree@vger.kernel.org; pawel.moll@arm.com; robh+dt@kernel.org; > linux-spi@vger.kernel.org; Mark Brown; linux-devel@gforge.freescale.net; > Scott Wood; Yang-Leo Li; shawnguo@kernel.org > Subject: Re: [linux-devel] [PATCH 1/2] spi: spi-fsl-dspi: replace regmap > R/W with internal implementation > > On Mon, May 9, 2016 at 5:58 AM, Yao Yuan <yao.yuan@nxp.com> wrote: > > On Mon, May 09, 2016 at 06:29:59AM +0800, Mark Brown wrote: > >> On Mon, May 09, 2016 at 08:48:59AM +0000, Yao Yuan wrote: > >> > Hi Mark Brown, > >> > >> Please don't top post, reply in line with needed context. This > >> allows readers to readily follow the flow of conversation and > >> understand what you are talking about and also helps ensure that > >> everything in the discussion is being addressed. > >> > >> Please fix your mail client to word wrap within paragraphs at > >> something substantially less than 80 columns. Doing this makes your > >> messages much easier to read and reply to. > >> > > > > Ok, Thanks. > > > >> > There are two problems we have found for use regmap up to now. > >> > >> > 1, Regmap will read device value depends on endian in .dtsi(DSPI in > >> > .dtsi file is > >> big-endian) and readl()(readl() in arm64 is le32_to_cpu read) in > system. > >> > That is the value read out is fixed regardless the kernel were > >> > compiled with > >> CONFIG_CPU_BIG_ENDIAN yes or no. That makes the value(read from > >> DSPI)can't be recognized in one endian mode(CONFIG_CPU_BIG_ENDIAN be > >> set or not). > >> > The result is: DSPI can't working with CONFIG_CPU_BIG_ENDIAN set. > >> > >> This doesn't make sense. If the endianness of the device is > >> specified in DT then the endianness of the CPU won't make a > difference. > >> > > > > For example, if DSPI controller register is BE, so I set big-endian in > DT. > > That means we should R/W the DSPI controller register with big-endian. > > Then I can think all of the value we R/W to/from DSPI controller > > register, we can Think they are the BE. > > So that we can understand it easy. > > > > That means no matter core is LE or BE, we both need get the value from > > DSPI register with big endian. > > > > But from regmap is not. > > When the core is little endian, I can get the big endian value form > register like:0xaabbccdd. > > Then we can analysis and process this value. > > But once config the core to big endian, I get the value > like:0xddccbbaa. It's the little endian. > > That's terrible for my driver to understand the value. > > This is just the result. Are you able to dig deeper to explain why this > is happening? spi/spi-fsl-dspi.c use the devm_regmap_init_mmio_clk() get regmap which will use regmap_mmio for operation the registers. static struct regmap_bus regmap_mmio = { ... .write = regmap_mmio_write, .read = regmap_mmio_read, } Anyway, the whole process register read operation will include regmap_mmio_read and format.parse_val(map->work_buf) then get the value. regmap_mmio_read() will use le32_to_cpu() value, format.parse_val() will use cpu_to_be32() (if .dtsi file config the dspi with big_endian, else use cpu_to_le32() if set little_endian ) So for kernel we get the value will always le32_to_cpu() + cpu_to_be32(), in short, the value always get value from register le32_to_be32 read. This operation could not be recognized for both big_endian and little_endian core setting. Founded that it will hang in one type of CPU ENDIAN mode. > > I checked the LS1043 reference manual, the register definition looks > like little endian instead of the big endian defined in the LS1043 > device tree. Can you confirm if the block is actually having big endian > registers or little endian registers. > > Regards, > Leo > _______________________________________________ > linux-devel mailing list > linux-devel@gforge.freescale.net > http://gforge.freescale.net/mailman/listinfo/linux-devel
On Mon, May 09, 2016 at 10:58:08AM +0000, Yao Yuan wrote: > For example, if DSPI controller register is BE, so I set big-endian in DT. > That means we should R/W the DSPI controller register with big-endian. > Then I can think all of the value we R/W to/from DSPI controller register, we can > Think they are the BE. > So that we can understand it easy. > That means no matter core is LE or BE, we both need get the value from DSPI register > with big endian. That's not how regmap is intended to be used, the intention is that the driver will interact with native endian values and regmap will hide the endianness changes. You could mask this with some cpu_to_be() and be_to_cpu() usage but I'm not sure it's a great idea or what it's buying you.
On Thu, May 12, 2016 at 11:31 AM, Mark Brown <broonie@kernel.org> wrote: > On Mon, May 09, 2016 at 10:58:08AM +0000, Yao Yuan wrote: > >> For example, if DSPI controller register is BE, so I set big-endian in DT. >> That means we should R/W the DSPI controller register with big-endian. >> Then I can think all of the value we R/W to/from DSPI controller register, we can >> Think they are the BE. >> So that we can understand it easy. > >> That means no matter core is LE or BE, we both need get the value from DSPI register >> with big endian. > > That's not how regmap is intended to be used, the intention is that the > driver will interact with native endian values and regmap will hide the > endianness changes. You could mask this with some cpu_to_be() and > be_to_cpu() usage but I'm not sure it's a great idea or what it's buying > you. So you do agree that the DSPI driver shouldn't use the regmap APIs? It is still confusing to me on what is the intended behavior for the endianness property in device tree. Previously for regmap binding the default endian is the native-endian if no endian property is not present. But for bindings of many devices including the Documentation/devicetree/bindings/common-properties.txt file, the default endian is the endian when the device is first used in the dts binding. For example, the FSL/NXP IFC device was first used as a big-endian device for PowerPC SoCs. The default endian should be big-endian no matter if it is used on PowerPC or ARM later. It shouldn't be default to little-endian on an ARM SoC. Regards, Leo -- To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, May 12, 2016 at 06:55:38PM -0500, Leo Li wrote: > On Thu, May 12, 2016 at 11:31 AM, Mark Brown <broonie@kernel.org> wrote: > > On Mon, May 09, 2016 at 10:58:08AM +0000, Yao Yuan wrote: > > That's not how regmap is intended to be used, the intention is that the > > driver will interact with native endian values and regmap will hide the > > endianness changes. You could mask this with some cpu_to_be() and > > be_to_cpu() usage but I'm not sure it's a great idea or what it's buying > > you. > So you do agree that the DSPI driver shouldn't use the regmap APIs? No, of course not! The driver's usage just appears to be very confused. > It is still confusing to me on what is the intended behavior for the > endianness property in device tree. Previously for regmap binding the What is confusing? If the device is big endian the driver should specify big endian. If the device is little endian the driver should specify little endian. If the device is native endian the driver should specify native endian. Regardless of what the endianness of the device is the driver should always use native endian when talking to the regmap APIs. > default endian is the native-endian if no endian property is not > present. But for bindings of many devices including the No, the default is little endian (it always has been, though for a long time only by accident and now the documentation matches that). If this was causing problems you should have reported it rather than trying to hack around it.
On 05/12/2016 11:31 AM, Mark Brown wrote: > On Mon, May 09, 2016 at 10:58:08AM +0000, Yao Yuan wrote: > >> For example, if DSPI controller register is BE, so I set big-endian in DT. >> That means we should R/W the DSPI controller register with big-endian. >> Then I can think all of the value we R/W to/from DSPI controller register, we can >> Think they are the BE. >> So that we can understand it easy. > >> That means no matter core is LE or BE, we both need get the value from DSPI register >> with big endian. > > That's not how regmap is intended to be used, the intention is that the > driver will interact with native endian values and regmap will hide the > endianness changes. You could mask this with some cpu_to_be() and > be_to_cpu() usage but I'm not sure it's a great idea or what it's buying > you. I don't think that's what Yao Yuan meant. The driver wants the I/O accessor to return native-endian values, just like any other driver. -Scott -- To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 05/09/2016 05:29 AM, Mark Brown wrote: > On Mon, May 09, 2016 at 08:48:59AM +0000, Yao Yuan wrote: >> 2, Sometimes regmap will generate some issues. Some regmap issues will cause the DSPI do not working. >> At 2016.4 we find the DSPI was not work, then we found that the regmap R/W the controller register sometimes not right. >> The result is: DSPI can't working with REGMAP API. >> Although the regmap issues was fast fixed by some giving person. But I'm still tend to use the reliable way. > >> Regmap is nice and Maybe I should try to fix regmap instead of replacing regmap in my driver. >> But regmap is really very large and complex for DSPI driver. > > Yes, if you find bugs in generic frameworks you should fix them rather > than just open coding something. That's often true, but using regmap to handle endianness is like swatting flies with a sledgehammer. Regmap's code is quite (over?) complicated (e.g. the (user-visible!) difference between reg_format_endian and val_format_endian is quite confusing) and ideally people who aren't getting actual value from a buggy and/or awkward subsystem shouldn't be forced to use/fix it. We're not talking about massive duplication here. We're talking about a set of simple I/O wrappers (something that many drivers do) with an if-statement. That said, Yao Yuan, can you try linux-next to see if the regmap problems have been fixed? Looking at Linus's tree I don't see how regmap-mmio would ever give you anything but little-endian if the driver doesn't specify an endianness (the device tree is only looked at by regmap_get_val_endian() which wasn't called for regmap-mmio), but that appears to be fixed in linux-next. -Scott -- To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, May 12, 2016 at 09:41 OM, Po Liu wrote: > On Thu, May12, 2016 at 06:20 AM, Leo Li Wrote: > > On Mon, May 9, 2016 at 5:58 AM, Yao Yuan <yao.yuan@nxp.com> wrote: > > > On Mon, May 09, 2016 at 06:29:59AM +0800, Mark Brown wrote: > > >> On Mon, May 09, 2016 at 08:48:59AM +0000, Yao Yuan wrote: > > >> > Hi Mark Brown, > > >> > > >> Please don't top post, reply in line with needed context. This > > >> allows readers to readily follow the flow of conversation and >> > > understand what you are talking about and also helps ensure that >> > > everything in the discussion is being addressed. > > >> > > >> Please fix your mail client to word wrap within paragraphs at >> > > something substantially less than 80 columns. Doing this makes your > > >> messages much easier to read and reply to. > > >> > > > > > > Ok, Thanks. > > > > > >> > There are two problems we have found for use regmap up to now. > > >> > > >> > 1, Regmap will read device value depends on endian in .dtsi(DSPI > > in >> > .dtsi file is >> big-endian) and readl()(readl() in arm64 is > > le32_to_cpu read) in system. > > >> > That is the value read out is fixed regardless the kernel were > > >> > compiled with >> CONFIG_CPU_BIG_ENDIAN yes or no. That makes the > > value(read from >> DSPI)can't be recognized in one endian > > mode(CONFIG_CPU_BIG_ENDIAN be >> set or not). > > >> > The result is: DSPI can't working with CONFIG_CPU_BIG_ENDIAN set. > > >> > > >> This doesn't make sense. If the endianness of the device is >> > > specified in DT then the endianness of the CPU won't make a > > difference. > > >> > > > > > > For example, if DSPI controller register is BE, so I set big-endian > > in DT. > > > That means we should R/W the DSPI controller register with big-endian. > > > Then I can think all of the value we R/W to/from DSPI controller > > > register, we can Think they are the BE. > > > So that we can understand it easy. > > > > > > That means no matter core is LE or BE, we both need get the value > > from > DSPI register with big endian. > > > > > > But from regmap is not. > > > When the core is little endian, I can get the big endian value form > > register like:0xaabbccdd. > > > Then we can analysis and process this value. > > > But once config the core to big endian, I get the value > > like:0xddccbbaa. It's the little endian. > > > That's terrible for my driver to understand the value. > > > > This is just the result. Are you able to dig deeper to explain why > > this is happening? > spi/spi-fsl-dspi.c use the devm_regmap_init_mmio_clk() get regmap which will > use regmap_mmio for operation the registers. > > static struct regmap_bus regmap_mmio = { > ... > .write = regmap_mmio_write, > .read = regmap_mmio_read, > } > > Anyway, the whole process register read operation will include > regmap_mmio_read and format.parse_val(map->work_buf) then get the > value. > regmap_mmio_read() will use le32_to_cpu() value, > format.parse_val() will use cpu_to_be32() (if .dtsi file config the dspi > with big_endian, else use cpu_to_le32() if set little_endian ) > > So for kernel we get the value will always le32_to_cpu() + cpu_to_be32(), in > short, the value always get value from register le32_to_be32 read. This > operation could not be recognized for both big_endian and little_endian core > setting. Founded that it will hang in one type of CPU ENDIAN mode. Yes, That's the reason, Thanks for you to explain. > > > > > I checked the LS1043 reference manual, the register definition looks > > like little endian instead of the big endian defined in the LS1043 > > device tree. Can you confirm if the block is actually having big > > endian registers or little endian registers. No, it should be the document issue. The register for DSPI on LS1043A is still big-endian. Maybe will changed to little-endian in some later new SOC. > > > > Regards, > > Leo > > _______________________________________________ > > linux-devel mailing list > > linux-devel@gforge.freescale.net > > http://gforge.freescale.net/mailman/listinfo/linux-devel
On Fri, May 13, 2016 at 02:29:03AM +0000, Scott Wood wrote: > On 05/12/2016 11:31 AM, Mark Brown wrote: > >> That means no matter core is LE or BE, we both need get the value from DSPI register > >> with big endian. > > That's not how regmap is intended to be used, the intention is that the > > driver will interact with native endian values and regmap will hide the > > endianness changes. You could mask this with some cpu_to_be() and > > be_to_cpu() usage but I'm not sure it's a great idea or what it's buying > > you. > I don't think that's what Yao Yuan meant. The driver wants the I/O > accessor to return native-endian values, just like any other driver. This really isn't clear to me.
On Fri, May 13, 2016 at 03:08:55AM +0000, Scott Wood wrote: > On 05/09/2016 05:29 AM, Mark Brown wrote: > > Yes, if you find bugs in generic frameworks you should fix them rather > > than just open coding something. > That's often true, but using regmap to handle endianness is like > swatting flies with a sledgehammer. Regmap's code is quite (over?) > complicated (e.g. the (user-visible!) difference between > reg_format_endian and val_format_endian is quite confusing) and ideally This is the register map API, not the register API - many buses, including MMIO, have different endiannesses for their values and addresses so of course we need to let users control that. > people who aren't getting actual value from a buggy and/or awkward > subsystem shouldn't be forced to use/fix it. We're not talking about > massive duplication here. We're talking about a set of simple I/O > wrappers (something that many drivers do) with an if-statement. As far as I am aware any issues have been fixed by other users who reported the problems they were seeing (which hasn't happened here). When we encounter problems it is not OK to just bodge around the issue in a driver without reporting them. That doesn't help other users and makes everything more fragile. If there were some history or other indication of real problems here the story would be different but if that's been happening it hasn't ever been communicated. As it is it looks like people have been using the diagnostic tools (max_register is specified) and clock management provided by regmap and the way this has been communicated is causing me concern.
diff --git a/drivers/spi/spi-fsl-dspi.c b/drivers/spi/spi-fsl-dspi.c index 9e9dadb..9a61572 100644 --- a/drivers/spi/spi-fsl-dspi.c +++ b/drivers/spi/spi-fsl-dspi.c @@ -27,7 +27,6 @@ #include <linux/pinctrl/consumer.h> #include <linux/platform_device.h> #include <linux/pm_runtime.h> -#include <linux/regmap.h> #include <linux/sched.h> #include <linux/spi/spi.h> #include <linux/spi/spi_bitbang.h> @@ -143,7 +142,7 @@ struct fsl_dspi { struct spi_master *master; struct platform_device *pdev; - struct regmap *regmap; + void __iomem *iobase; int irq; struct clk *clk; @@ -165,13 +164,46 @@ struct fsl_dspi { u32 waitflags; u32 spi_tcnt; + bool big_endian; }; +/* + * R/W functions for big- or little-endian registers: + * The qSPI controller's endian is independent of the CPU core's endian. + */ +static void dspi_writel(struct fsl_dspi *d, u32 val, u32 offset) +{ + if (d->big_endian) + iowrite32be(val, d->iobase + offset); + else + iowrite32(val, d->iobase + offset); +} + +static u32 dspi_readl(struct fsl_dspi *d, u32 offset) +{ + if (d->big_endian) + return ioread32be(d->iobase + offset); + else + return ioread32(d->iobase + offset); +} + +static void dspi_updatel(struct fsl_dspi *d, u32 mask, u32 val, u32 offset) +{ + u32 tmp, orig; + + orig = dspi_readl(d, offset); + + tmp = orig & ~mask; + tmp |= val & mask; + + dspi_writel(d, tmp, offset); +} + static inline int is_double_byte_mode(struct fsl_dspi *dspi) { unsigned int val; - regmap_read(dspi->regmap, SPI_CTAR(0), &val); + val = dspi_readl(dspi, SPI_CTAR(0)); return ((val & SPI_FRAME_BITS_MASK) == SPI_FRAME_BITS(8)) ? 0 : 1; } @@ -270,7 +302,7 @@ static void dspi_data_from_popr(struct fsl_dspi *dspi, int rx_word) u16 d; unsigned int val; - regmap_read(dspi->regmap, SPI_POPR, &val); + val = dspi_readl(dspi, SPI_POPR); d = SPI_POPR_RXDATA(val); if (!(dspi->dataflags & TRAN_STATE_RX_VOID)) @@ -294,8 +326,8 @@ static int dspi_eoq_write(struct fsl_dspi *dspi) */ if (tx_word && (dspi->len == 1)) { dspi->dataflags |= TRAN_STATE_WORD_ODD_NUM; - regmap_update_bits(dspi->regmap, SPI_CTAR(0), - SPI_FRAME_BITS_MASK, SPI_FRAME_BITS(8)); + dspi_updatel(dspi, SPI_FRAME_BITS_MASK, + SPI_FRAME_BITS(8), SPI_CTAR(0)); tx_word = 0; } @@ -309,7 +341,7 @@ static int dspi_eoq_write(struct fsl_dspi *dspi) } else if (tx_word && (dspi->len == 1)) dspi_pushr |= SPI_PUSHR_EOQ; - regmap_write(dspi->regmap, SPI_PUSHR, dspi_pushr); + dspi_writel(dspi, dspi_pushr, SPI_PUSHR); tx_count++; } @@ -343,8 +375,8 @@ static int dspi_tcfq_write(struct fsl_dspi *dspi) if (tx_word && (dspi->len == 1)) { dspi->dataflags |= TRAN_STATE_WORD_ODD_NUM; - regmap_update_bits(dspi->regmap, SPI_CTAR(0), - SPI_FRAME_BITS_MASK, SPI_FRAME_BITS(8)); + dspi_updatel(dspi, SPI_FRAME_BITS_MASK, + SPI_FRAME_BITS(8), SPI_CTAR(0)); tx_word = 0; } @@ -353,7 +385,7 @@ static int dspi_tcfq_write(struct fsl_dspi *dspi) if ((dspi->cs_change) && (!dspi->len)) dspi_pushr &= ~SPI_PUSHR_CONT; - regmap_write(dspi->regmap, SPI_PUSHR, dspi_pushr); + dspi_writel(dspi, dspi_pushr, SPI_PUSHR); return tx_word + 1; } @@ -378,7 +410,7 @@ static int dspi_transfer_one_message(struct spi_master *master, enum dspi_trans_mode trans_mode; u32 spi_tcr; - regmap_read(dspi->regmap, SPI_TCR, &spi_tcr); + spi_tcr = dspi_readl(dspi, SPI_TCR); dspi->spi_tcnt = SPI_TCR_GET_TCNT(spi_tcr); message->actual_length = 0; @@ -407,21 +439,19 @@ static int dspi_transfer_one_message(struct spi_master *master, if (!dspi->tx) dspi->dataflags |= TRAN_STATE_TX_VOID; - regmap_write(dspi->regmap, SPI_MCR, dspi->cur_chip->mcr_val); - regmap_update_bits(dspi->regmap, SPI_MCR, - SPI_MCR_CLR_TXF | SPI_MCR_CLR_RXF, - SPI_MCR_CLR_TXF | SPI_MCR_CLR_RXF); - regmap_write(dspi->regmap, SPI_CTAR(0), - dspi->cur_chip->ctar_val); + dspi_writel(dspi, dspi->cur_chip->mcr_val, SPI_MCR); + dspi_updatel(dspi, SPI_MCR_CLR_TXF | SPI_MCR_CLR_RXF, + SPI_MCR_CLR_TXF | SPI_MCR_CLR_RXF, SPI_MCR); + dspi_writel(dspi, dspi->cur_chip->ctar_val, SPI_CTAR(0)); trans_mode = dspi->devtype_data->trans_mode; switch (trans_mode) { case DSPI_EOQ_MODE: - regmap_write(dspi->regmap, SPI_RSER, SPI_RSER_EOQFE); + dspi_writel(dspi, SPI_RSER_EOQFE, SPI_RSER); dspi_eoq_write(dspi); break; case DSPI_TCFQ_MODE: - regmap_write(dspi->regmap, SPI_RSER, SPI_RSER_TCFQE); + dspi_writel(dspi, SPI_RSER_TCFQE, SPI_RSER); dspi_tcfq_write(dspi); break; default: @@ -525,14 +555,14 @@ static irqreturn_t dspi_interrupt(int irq, void *dev_id) u32 spi_tcnt, tcnt_diff; int tx_word; - regmap_read(dspi->regmap, SPI_SR, &spi_sr); - regmap_write(dspi->regmap, SPI_SR, spi_sr); + spi_sr = dspi_readl(dspi, SPI_SR); + dspi_writel(dspi, spi_sr, SPI_SR); if (spi_sr & (SPI_SR_EOQF | SPI_SR_TCFQF)) { tx_word = is_double_byte_mode(dspi); - regmap_read(dspi->regmap, SPI_TCR, &spi_tcr); + spi_tcr = dspi_readl(dspi, SPI_TCR); spi_tcnt = SPI_TCR_GET_TCNT(spi_tcr); /* * The width of SPI Transfer Counter in SPI_TCR is 16bits, @@ -569,10 +599,10 @@ static irqreturn_t dspi_interrupt(int irq, void *dev_id) if (!dspi->len) { if (dspi->dataflags & TRAN_STATE_WORD_ODD_NUM) { - regmap_update_bits(dspi->regmap, - SPI_CTAR(0), - SPI_FRAME_BITS_MASK, - SPI_FRAME_BITS(16)); + dspi_updatel(dspi, + SPI_FRAME_BITS_MASK, + SPI_FRAME_BITS(16), + SPI_CTAR(0)); dspi->dataflags &= ~TRAN_STATE_WORD_ODD_NUM; } @@ -636,13 +666,6 @@ static int dspi_resume(struct device *dev) static SIMPLE_DEV_PM_OPS(dspi_pm, dspi_suspend, dspi_resume); -static const struct regmap_config dspi_regmap_config = { - .reg_bits = 32, - .val_bits = 32, - .reg_stride = 4, - .max_register = 0x88, -}; - static int dspi_probe(struct platform_device *pdev) { struct device_node *np = pdev->dev.of_node; @@ -700,13 +723,7 @@ static int dspi_probe(struct platform_device *pdev) goto out_master_put; } - dspi->regmap = devm_regmap_init_mmio_clk(&pdev->dev, NULL, base, - &dspi_regmap_config); - if (IS_ERR(dspi->regmap)) { - dev_err(&pdev->dev, "failed to init regmap: %ld\n", - PTR_ERR(dspi->regmap)); - return PTR_ERR(dspi->regmap); - } + dspi->iobase = base; dspi->irq = platform_get_irq(pdev, 0); if (dspi->irq < 0) { @@ -730,6 +747,8 @@ static int dspi_probe(struct platform_device *pdev) } clk_prepare_enable(dspi->clk); + dspi->big_endian = of_property_read_bool(np, "big-endian"); + master->max_speed_hz = clk_get_rate(dspi->clk) / dspi->devtype_data->max_clock_factor;