From 6edd0fa88631cc20a742e0567c2e0e6cb9e5a8fc Mon Sep 17 00:00:00 2001 From: Johannes Poeehlmann Date: Thu, 9 Feb 2017 14:05:22 +0100 Subject: [PATCH 1/4] fix and simplify register access o Replace incorrect register offsett calculation by direct configuration of bus_shift in mfd-cell. Indirect definition of address-shift by resource size was unobvious and should have used a binary log. o Make endian clean, make HW-endianness configurable. o Use ioread*, iowrite* instead of __raw_readb,__raw_writeb to also use memory-barriers when accessing HW-registers. We do not want reordering to happen here. Signed-off-by: Johannes Poeehlmann --- drivers/w1/masters/ds1wm.c | 92 +++++++++++++++++++++++++++++++++++++++++++--- include/linux/mfd/ds1wm.h | 10 +++++ 2 files changed, 97 insertions(+), 5 deletions(-) diff --git a/drivers/w1/masters/ds1wm.c b/drivers/w1/masters/ds1wm.c index e0b8a4b..e6284b7 100644 --- a/drivers/w1/masters/ds1wm.c +++ b/drivers/w1/masters/ds1wm.c @@ -97,6 +97,7 @@ static struct { struct ds1wm_data { void __iomem *map; int bus_shift; /* # of shifts to calc register offsets */ + int isHwBigEndian; struct platform_device *pdev; const struct mfd_cell *cell; int irq; @@ -116,12 +117,83 @@ struct ds1wm_data { static inline void ds1wm_write_register(struct ds1wm_data *ds1wm_data, u32 reg, u8 val) { - __raw_writeb(val, ds1wm_data->map + (reg << ds1wm_data->bus_shift)); + if (ds1wm_data->isHwBigEndian) { + switch (ds1wm_data->bus_shift) { + case BUSWIDTH8: + writeb(val, ds1wm_data->map + (reg << BUSWIDTH8)); + break; + case BUSWIDTH16: + writew_be((u16)val, ds1wm_data->map+(reg<map+(reg<pdev->dev, + "illegal bus shift %d, not written", + ds1wm_data->bus_shift); + } + } else { + switch (ds1wm_data->bus_shift) { + case BUSWIDTH8: + writeb(val, ds1wm_data->map + (reg << BUSWIDTH8)); + break; + case BUSWIDTH16: + writew((u16) val, ds1wm_data->map+(reg << BUSWIDTH16)); + break; + case BUSWIDTH32: + writel((u32) val, ds1wm_data->map+(reg << BUSWIDTH32)); + break; + default: + dev_err(&ds1wm_data->pdev->dev, + "illegal bus shift %d, not written", + ds1wm_data->bus_shift); + } + } } static inline u8 ds1wm_read_register(struct ds1wm_data *ds1wm_data, u32 reg) { - return __raw_readb(ds1wm_data->map + (reg << ds1wm_data->bus_shift)); + + u32 val = 0; + if (ds1wm_data->isHwBigEndian) { + switch (ds1wm_data->bus_shift) { + case BUSWIDTH8: + val = readb(ds1wm_data->map + (reg << BUSWIDTH8)); + break; + case BUSWIDTH16: + val = readw_be(ds1wm_data->map + (reg << BUSWIDTH16)); + break; + case BUSWIDTH32: + val = readl_be(ds1wm_data->map + (reg << BUSWIDTH32)); + break; + default: + dev_err(&ds1wm_data->pdev->dev, + "illegal bus shift %d, not read", + ds1wm_data->bus_shift); + } + } else { + switch (ds1wm_data->bus_shift) { + case BUSWIDTH8: + val = readb(ds1wm_data->map + (reg << BUSWIDTH8)); + break; + case BUSWIDTH16: + val = readw(ds1wm_data->map + (reg << BUSWIDTH16)); + break; + case BUSWIDTH32: + val = readl(ds1wm_data->map + (reg << BUSWIDTH32)); + break; + default: + dev_err(&ds1wm_data->pdev->dev, + "illegal bus shift %d, not read", + ds1wm_data->bus_shift); + + return 0; + } + } + dev_dbg(&ds1wm_data->pdev->dev, + "ds1wm_read_register reg: %d, 32 bit val:%x\n", reg, val); + return (u8) val; } @@ -474,9 +546,6 @@ static int ds1wm_probe(struct platform_device *pdev) if (!ds1wm_data->map) return -ENOMEM; - /* calculate bus shift from mem resource */ - ds1wm_data->bus_shift = resource_size(res) >> 3; - ds1wm_data->pdev = pdev; ds1wm_data->cell = mfd_get_cell(pdev); if (!ds1wm_data->cell) @@ -485,6 +554,19 @@ static int ds1wm_probe(struct platform_device *pdev) if (!plat) return -ENODEV; + /* how many bits to shift register number to get register offset */ + ds1wm_data->bus_shift = plat->bus_shift; + + /* make sure resource has space for 8 registers */ + if ( (8 << ds1wm_data->bus_shift ) > resource_size(res) ){ + dev_err(&ds1wm_data->pdev->dev, + "memory ressource size %d to small, should be %d\n", + (int) resource_size(res), + 8 << ds1wm_data->bus_shift); + } + + ds1wm_data->isHwBigEndian = plat->isHwBigEndian; + res = platform_get_resource(pdev, IORESOURCE_IRQ, 0); if (!res) return -ENXIO; diff --git a/include/linux/mfd/ds1wm.h b/include/linux/mfd/ds1wm.h index 38a372a..2fbc433 100644 --- a/include/linux/mfd/ds1wm.h +++ b/include/linux/mfd/ds1wm.h @@ -1,5 +1,10 @@ /* MFD cell driver data for the DS1WM driver */ +/* bus shift constants */ +#define BUSWIDTH8 0 +#define BUSWIDTH16 1 +#define BUSWIDTH32 2 + struct ds1wm_driver_data { int active_high; int clock_rate; @@ -10,4 +15,9 @@ struct ds1wm_driver_data { /* ds1wm implements the precise timings of*/ /* a reset pulse/presence detect sequence.*/ unsigned int reset_recover_delay; + /* Say 1 here for big endian Hardware + * (only relevant with bus-shift > 0*/ + int isHwBigEndian; + /* left shift of register number to get register address offsett */ + int bus_shift; }; -- 2.1.4