Re: [PATCH v2] staging: kpc2000: kpc_i2c: remove the macros inb_p and outb_p
On Mon, Jun 10, 2019 at 03:48:24PM +0800, Hao Xu wrote: > remove inb_p and outb_p to call readq/writeq directly. > > Signed-off-by: Hao Xu > --- > Changes in v2: > - remove the macros inb_p/outb_p and use readq/writeq directly, per > https://lkml.kernel.org/lkml/20190608134505.ga...@arch-01.home/ > --- > drivers/staging/kpc2000/kpc2000_i2c.c | 112 > -- > 1 file changed, 53 insertions(+), 59 deletions(-) > > diff --git a/drivers/staging/kpc2000/kpc2000_i2c.c > b/drivers/staging/kpc2000/kpc2000_i2c.c > index 69e8773..246d5b3 100644 > --- a/drivers/staging/kpc2000/kpc2000_i2c.c > +++ b/drivers/staging/kpc2000/kpc2000_i2c.c > @@ -307,28 +301,28 @@ static int i801_block_transaction_byte_by_byte(struct > i2c_device *priv, union i2 > else > smbcmd = I801_BLOCK_DATA; > } > - outb_p(smbcmd | ENABLE_INT9, SMBHSTCNT(priv)); > + writeq(smbcmd | ENABLE_INT9, (void *)SMBHSTCNT(priv)); > > if (i == 1) > - outb_p(inb(SMBHSTCNT(priv)) | I801_START, > SMBHSTCNT(priv)); > + writeq(inb(SMBHSTCNT(priv)) | I801_START, (void > *)SMBHSTCNT(priv)); This inb() call looks like a bug. We perform a 64-bit operation when talking to this hardware register everywhere else in this driver. Anyone have more insight into the hardware with which this driver interacts such that they could shed some light on the subject? Probably a separate issue, but I did notice it as a result of this patch. Thanks, Geordan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/2] staging: kpc2000: kpc2000_i2c: void* -> void *
On Sat, Jun 08, 2019 at 03:27:46PM +0800, Hao Xu wrote:
> modify void* to void * for #define inb_p(a) readq((void*)a)
> and #define outb_p(d,a) writeq(d,(void*)a)
>
> Signed-off-by: Hao Xu
> ---
> drivers/staging/kpc2000/kpc2000_i2c.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/kpc2000/kpc2000_i2c.c
> b/drivers/staging/kpc2000/kpc2000_i2c.c
> index a434dd0..de3a0c8 100644
> --- a/drivers/staging/kpc2000/kpc2000_i2c.c
> +++ b/drivers/staging/kpc2000/kpc2000_i2c.c
> @@ -124,9 +124,9 @@ struct i2c_device {
>
> // FIXME!
> #undef inb_p
> -#define inb_p(a) readq((void*)a)
> +#define inb_p(a) readq((void *)a)
> #undef outb_p
> -#define outb_p(d,a) writeq(d,(void*)a)
> +#define outb_p(d,a) writeq(d,(void *)a)
Alternatively to fixing up the style here, did you consider just
removing these two macros altogether and calling [read|write]q
directly throughout the kpc_i2c driver (per the '//FIXME' comment)?
Unless, I'm misunderstanding something, these macros are shadowing the
functions [in|out]b_p, which already exist in io.h. [in|out]b_p are for
8-bit i/o transactions and [read|write]q are for 64-bit transactions, so
shadowing the original [in|out]b_p with something that actually does
64-bit transactions is probably potentially misleading here.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 4/6] staging: kpc2000: kpc_spi: remove unnecessary struct member chip_select
The structure kp_spi_controller_state, defined in the kpc2000_spi
driver, contains a member named chip_select which is never used after
initialization. Therefore, it should be removed for simplicity's sake.
Signed-off-by: Geordan Neukum
---
drivers/staging/kpc2000/kpc2000_spi.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/staging/kpc2000/kpc2000_spi.c
b/drivers/staging/kpc2000/kpc2000_spi.c
index 61296335313b..07b0327d8bef 100644
--- a/drivers/staging/kpc2000/kpc2000_spi.c
+++ b/drivers/staging/kpc2000/kpc2000_spi.c
@@ -109,7 +109,6 @@ struct kp_spi {
struct kp_spi_controller_state {
void __iomem *base;
- unsigned char chip_select;
s64 conf_cache;
};
@@ -267,7 +266,6 @@ kp_spi_setup(struct spi_device *spidev)
return -ENOMEM;
}
cs->base = kpspi->base;
- cs->chip_select = spidev->chip_select;
cs->conf_cache = -1;
spidev->controller_state = cs;
}
--
2.21.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 6/6] staging: kpc2000: kpc_spi: remove unnecessary cast in [read|write]_reg()
The kpc_spi driver unnecessarily casts from a (u64 __iomem *) to a (void
*) when invoking readq and writeq which both take a (void __iomem *) arg.
There is no need for this cast, and it actually harms us by discarding
the sparse cookie, __iomem. Make the driver stop performing this casting
operation.
Signed-off-by: Geordan Neukum
---
drivers/staging/kpc2000/kpc2000_spi.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/kpc2000/kpc2000_spi.c
b/drivers/staging/kpc2000/kpc2000_spi.c
index 4f517afc6239..28132e9e260d 100644
--- a/drivers/staging/kpc2000/kpc2000_spi.c
+++ b/drivers/staging/kpc2000/kpc2000_spi.c
@@ -167,7 +167,7 @@ kp_spi_read_reg(struct kp_spi_controller_state *cs, int idx)
if ((idx == KP_SPI_REG_CONFIG) && (cs->conf_cache >= 0)){
return cs->conf_cache;
}
- val = readq((void*)addr);
+ val = readq(addr);
return val;
}
@@ -176,7 +176,7 @@ kp_spi_write_reg(struct kp_spi_controller_state *cs, int
idx, u64 val)
{
u64 __iomem *addr = cs->base;
addr += idx;
- writeq(val, (void*)addr);
+ writeq(val, addr);
if (idx == KP_SPI_REG_CONFIG)
cs->conf_cache = val;
}
--
2.21.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/6] staging: kpc2000: kpc_spi: remove unnecessary struct member phys
The structure kp_spi_controller_state, defined in the kpc2000_spi
driver, contains a member named phys which is never used after
initialization. Therefore, it should be removed for simplicity's sake.
Signed-off-by: Geordan Neukum
---
drivers/staging/kpc2000/kpc2000_spi.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/staging/kpc2000/kpc2000_spi.c
b/drivers/staging/kpc2000/kpc2000_spi.c
index 32d3ec532e26..20c396bcd904 100644
--- a/drivers/staging/kpc2000/kpc2000_spi.c
+++ b/drivers/staging/kpc2000/kpc2000_spi.c
@@ -110,7 +110,6 @@ struct kp_spi {
struct kp_spi_controller_state {
void __iomem *base;
- unsigned long phys;
unsigned char chip_select;
int word_len;
s64 conf_cache;
@@ -270,7 +269,6 @@ kp_spi_setup(struct spi_device *spidev)
return -ENOMEM;
}
cs->base = kpspi->base;
- cs->phys = kpspi->phys;
cs->chip_select = spidev->chip_select;
cs->word_len = spidev->bits_per_word;
cs->conf_cache = -1;
--
2.21.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 3/6] staging: kpc2000: kpc_spi: remove unnecessary struct member word_len
The structure kp_spi_controller_state, defined in the kpc2000_spi
driver, contains a member named word_len which is never used after
initialization. Therefore, it should be removed for simplicity's sake.
Signed-off-by: Geordan Neukum
---
drivers/staging/kpc2000/kpc2000_spi.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/staging/kpc2000/kpc2000_spi.c
b/drivers/staging/kpc2000/kpc2000_spi.c
index 1d89cb3b861f..61296335313b 100644
--- a/drivers/staging/kpc2000/kpc2000_spi.c
+++ b/drivers/staging/kpc2000/kpc2000_spi.c
@@ -110,7 +110,6 @@ struct kp_spi {
struct kp_spi_controller_state {
void __iomem *base;
unsigned char chip_select;
- int word_len;
s64 conf_cache;
};
@@ -269,7 +268,6 @@ kp_spi_setup(struct spi_device *spidev)
}
cs->base = kpspi->base;
cs->chip_select = spidev->chip_select;
- cs->word_len = spidev->bits_per_word;
cs->conf_cache = -1;
spidev->controller_state = cs;
}
@@ -369,7 +367,6 @@ kp_spi_transfer_one_message(struct spi_master *master,
struct spi_message *m)
if (transfer->bits_per_word) {
word_len = transfer->bits_per_word;
}
- cs->word_len = word_len;
sc.bitfield.wl = word_len-1;
/* ...chip select */
--
2.21.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 5/6] staging: kpc2000: kpc_spi: remove unnecessary ulong repr of i/o addr
The kpc_spi driver stashes off an unsigned long representation of the
i/o mapping returned by devm_ioremap_nocache(). This is unnecessary, as
the only use of the unsigned long repr is to eventually be re-cast to
an (u64 __iomem *). Instead of casting the (void __iomem *) to an
(unsigned long) then a (u64 __iomem *), just remove this intermediate
step. As this intermediary is no longer used, also remove it from its
structure.
Signed-off-by: Geordan Neukum
---
drivers/staging/kpc2000/kpc2000_spi.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/kpc2000/kpc2000_spi.c
b/drivers/staging/kpc2000/kpc2000_spi.c
index 07b0327d8bef..4f517afc6239 100644
--- a/drivers/staging/kpc2000/kpc2000_spi.c
+++ b/drivers/staging/kpc2000/kpc2000_spi.c
@@ -103,7 +103,6 @@ static struct spi_board_info p2kr0_board_info[] = {
struct kp_spi {
struct spi_master *master;
u64 __iomem*base;
- unsigned long phys;
struct device *dev;
};
@@ -462,9 +461,8 @@ kp_spi_probe(struct platform_device *pldev)
goto free_master;
}
- kpspi->phys = (unsigned long)devm_ioremap_nocache(>dev, r->start,
- resource_size(r));
- kpspi->base = (u64 __iomem *)kpspi->phys;
+ kpspi->base = devm_ioremap_nocache(>dev, r->start,
+ resource_size(r));
status = spi_register_master(master);
if (status < 0) {
--
2.21.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 0/6] staging: kpc2000: kpc_spi: Assorted minor fixups
Primarily just a bunch of unused / unnecessarily used struct member cleanup patches with the exception of one patch which removes an unnecessary cast to a (void *) in a couple of functions. Geordan Neukum (6): staging: kpc2000: kpc_spi: remove unnecessary struct member phys staging: kpc2000: kpc_spi: remove unnecessary struct member pin_dir staging: kpc2000: kpc_spi: remove unnecessary struct member word_len staging: kpc2000: kpc_spi: remove unnecessary struct member chip_select staging: kpc2000: kpc_spi: remove unnecessary ulong repr of i/o addr staging: kpc2000: kpc_spi: remove unnecessary cast in [read|write]_reg() drivers/staging/kpc2000/kpc2000_spi.c | 19 --- 1 file changed, 4 insertions(+), 15 deletions(-) -- 2.21.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/6] staging: kpc2000: kpc_spi: remove unnecessary struct member pin_dir
The structure kpc_spi, defined in in the kpc2000_spi driver, contains
a member named pin_dir which is never used after initialization.
Therefore, it should be removed for simplicity's sake.
Signed-off-by: Geordan Neukum
---
drivers/staging/kpc2000/kpc2000_spi.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/staging/kpc2000/kpc2000_spi.c
b/drivers/staging/kpc2000/kpc2000_spi.c
index 20c396bcd904..1d89cb3b861f 100644
--- a/drivers/staging/kpc2000/kpc2000_spi.c
+++ b/drivers/staging/kpc2000/kpc2000_spi.c
@@ -105,7 +105,6 @@ struct kp_spi {
u64 __iomem*base;
unsigned long phys;
struct device *dev;
- unsigned intpin_dir:1;
};
struct kp_spi_controller_state {
@@ -460,7 +459,6 @@ kp_spi_probe(struct platform_device *pldev)
if (pldev->id != -1) {
master->bus_num = pldev->id;
}
- kpspi->pin_dir = 0;
r = platform_get_resource(pldev, IORESOURCE_MEM, 0);
if (r == NULL) {
--
2.21.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 3/5] staging: kpc2000: kpc_spi: remove fifo_depth from kp_spi struct
The kp_spi structure contains a member 'fifo_depth'. This member is
never used. Therefore, it should be removed.
Signed-off-by: Geordan Neukum
---
drivers/staging/kpc2000/kpc2000_spi.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/staging/kpc2000/kpc2000_spi.c
b/drivers/staging/kpc2000/kpc2000_spi.c
index 13c4651e1fac..049b1e324031 100644
--- a/drivers/staging/kpc2000/kpc2000_spi.c
+++ b/drivers/staging/kpc2000/kpc2000_spi.c
@@ -105,7 +105,6 @@ struct kp_spi {
u64 __iomem*base;
unsigned long phys;
struct device *dev;
- int fifo_depth;
unsigned intpin_dir:1;
};
--
2.21.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 4/5] staging: kpc2000: kpc_spi: remove function kp_spi_bytes_per_word()
The static function kp_spi_bytes_per_word() is defined in kpc2000_spi.c,
but it is completely unused. As this function is unused, it can and
should be removed.
Signed-off-by: Geordan Neukum
---
drivers/staging/kpc2000/kpc2000_spi.c | 14 --
1 file changed, 14 deletions(-)
diff --git a/drivers/staging/kpc2000/kpc2000_spi.c
b/drivers/staging/kpc2000/kpc2000_spi.c
index 049b1e324031..b513432a26ed 100644
--- a/drivers/staging/kpc2000/kpc2000_spi.c
+++ b/drivers/staging/kpc2000/kpc2000_spi.c
@@ -162,20 +162,6 @@ union kp_spi_ffctrl {
/***
* SPI Helpers *
***/
- static inline int
-kp_spi_bytes_per_word(int word_len)
-{
- if (word_len <= 8){
- return 1;
- }
- else if (word_len <= 16) {
- return 2;
- }
- else { /* word_len <= 32 */
- return 4;
- }
-}
-
static inline u64
kp_spi_read_reg(struct kp_spi_controller_state *cs, int idx)
{
--
2.21.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/5] staging: kpc2000: kpc_spi: column-align switch and subordinate cases
The linux style guide prescribes that switch statements and their
subordinate case labels should be column-aligned rather than
double-indenting the case label. Make kpc2000_spi.c follow the desired
style with respect to switch/case alignment.
Signed-off-by: Geordan Neukum
---
drivers/staging/kpc2000/kpc2000_spi.c | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/staging/kpc2000/kpc2000_spi.c
b/drivers/staging/kpc2000/kpc2000_spi.c
index ef7e062bf52c..13c4651e1fac 100644
--- a/drivers/staging/kpc2000/kpc2000_spi.c
+++ b/drivers/staging/kpc2000/kpc2000_spi.c
@@ -502,13 +502,13 @@ kp_spi_probe(struct platform_device *pldev)
}
switch ((drvdata->card_id & 0x) >> 16){
- case PCI_DEVICE_ID_DAKTRONICS_KADOKA_P2KR0:
- NEW_SPI_DEVICE_FROM_BOARD_INFO_TABLE(p2kr0_board_info);
- break;
- default:
- dev_err(>dev, "Unknown hardware, cant know what
partition table to use!\n");
- goto free_master;
- break;
+ case PCI_DEVICE_ID_DAKTRONICS_KADOKA_P2KR0:
+ NEW_SPI_DEVICE_FROM_BOARD_INFO_TABLE(p2kr0_board_info);
+ break;
+ default:
+ dev_err(>dev, "Unknown hardware, cant know what
partition table to use!\n");
+ goto free_master;
+ break;
}
return status;
--
2.21.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 5/5] staging: kpc2000: kpc_spi: use devm_* API to manage mapped I/O space
The kpc_spi driver does not unmap its I/O space upon error cases in the probe() function or upon remove(). Make the driver clean up after itself more maintainably by migrating to using the managed resource API. Signed-off-by: Geordan Neukum --- drivers/staging/kpc2000/kpc2000_spi.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/staging/kpc2000/kpc2000_spi.c b/drivers/staging/kpc2000/kpc2000_spi.c index b513432a26ed..32d3ec532e26 100644 --- a/drivers/staging/kpc2000/kpc2000_spi.c +++ b/drivers/staging/kpc2000/kpc2000_spi.c @@ -471,7 +471,8 @@ kp_spi_probe(struct platform_device *pldev) goto free_master; } - kpspi->phys = (unsigned long)ioremap_nocache(r->start, resource_size(r)); + kpspi->phys = (unsigned long)devm_ioremap_nocache(>dev, r->start, + resource_size(r)); kpspi->base = (u64 __iomem *)kpspi->phys; status = spi_register_master(master); -- 2.21.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 0/5] staging: kpc2000: kpc_spi: Assorted small fixes
This patch set contains a few small fixups to the kpc_spi driver. There is certainly nothing groundbreaking in this patch set. It is limited to style fixups, removing unused things, and using the managed resource API for mapping I/O space. Geordan Neukum (5): staging: kpc2000: kpc_spi: Remove unnecessary consecutive newlines staging: kpc2000: kpc_spi: column-align switch and subordinate cases staging: kpc2000: kpc_spi: remove fifo_depth from kp_spi struct staging: kpc2000: kpc_spi: remove function kp_spi_bytes_per_word() staging: kpc2000: kpc_spi: use devm_* API to manage mapped I/O space drivers/staging/kpc2000/kpc2000_spi.c | 45 ++- 1 file changed, 9 insertions(+), 36 deletions(-) -- 2.21.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/5] staging: kpc2000: kpc_spi: Remove unnecessary consecutive newlines
The kpc2000_spi.c file contains instances of unnecessary consecutive
newlines which negatively impact the readability of the file. Remove
all unnecessary consecutive newlines.
Signed-off-by: Geordan Neukum
---
drivers/staging/kpc2000/kpc2000_spi.c | 13 -
1 file changed, 13 deletions(-)
diff --git a/drivers/staging/kpc2000/kpc2000_spi.c
b/drivers/staging/kpc2000/kpc2000_spi.c
index 9a23808ffaa1..ef7e062bf52c 100644
--- a/drivers/staging/kpc2000/kpc2000_spi.c
+++ b/drivers/staging/kpc2000/kpc2000_spi.c
@@ -97,8 +97,6 @@ static struct spi_board_info p2kr0_board_info[] = {
#define KP_SPI_REG_STATUS_RXFFE 0x40
#define KP_SPI_REG_STATUS_RXFFF 0x80
-
-
/**
* SPI Structures *
**/
@@ -111,7 +109,6 @@ struct kp_spi {
unsigned intpin_dir:1;
};
-
struct kp_spi_controller_state {
void __iomem *base;
unsigned long phys;
@@ -120,7 +117,6 @@ struct kp_spi_controller_state {
s64 conf_cache;
};
-
union kp_spi_config {
/* use this to access individual elements */
struct __attribute__((packed)) spi_config_bitfield {
@@ -141,8 +137,6 @@ union kp_spi_config {
u32 reg;
};
-
-
union kp_spi_status {
struct __attribute__((packed)) spi_status_bitfield {
unsigned int rx: 1; /* Rx Status */
@@ -158,8 +152,6 @@ union kp_spi_status {
u32 reg;
};
-
-
union kp_spi_ffctrl {
struct __attribute__((packed)) spi_ffctrl_bitfield {
unsigned int ffstart : 1; /* FIFO Start */
@@ -168,8 +160,6 @@ union kp_spi_ffctrl {
u32 reg;
};
-
-
/***
* SPI Helpers *
***/
@@ -445,8 +435,6 @@ kp_spi_cleanup(struct spi_device *spidev)
}
}
-
-
/**
* Probe / Remove *
**/
@@ -538,7 +526,6 @@ kp_spi_remove(struct platform_device *pldev)
return 0;
}
-
static struct platform_driver kp_spi_driver = {
.driver = {
.name = KP_DRIVER_NAME_SPI,
--
2.21.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 7/8] staging: kpc2000: kpc_i2c: fail probe if unable to map I/O space
The kpc2000 driver does not verify whether or not mapping the I/O space succeeded during probe time. Make the driver verify that the mapping operation was successful before potentially using that area in the future. Signed-off-by: Geordan Neukum --- drivers/staging/kpc2000/kpc2000_i2c.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/staging/kpc2000/kpc2000_i2c.c b/drivers/staging/kpc2000/kpc2000_i2c.c index 452052bf9476..51e91653e183 100644 --- a/drivers/staging/kpc2000/kpc2000_i2c.c +++ b/drivers/staging/kpc2000/kpc2000_i2c.c @@ -591,6 +591,8 @@ static int pi2c_probe(struct platform_device *pldev) return -ENXIO; priv->smba = (unsigned long)ioremap_nocache(res->start, resource_size(res)); + if (!priv->smba) + return -ENOMEM; platform_set_drvdata(pldev, priv); -- 2.21.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 6/8] staging: kpc2000: kpc_i2c: fail probe if unable to get I/O resource
The kpc_i2c driver attempts to map its I/O space without verifying whether or not the result of platform_get_resource() is NULL. Make the driver check that platform_get_resource did not return NULL before attempting to use the value returned to map an I/O space. Signed-off-by: Geordan Neukum --- drivers/staging/kpc2000/kpc2000_i2c.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/staging/kpc2000/kpc2000_i2c.c b/drivers/staging/kpc2000/kpc2000_i2c.c index e4bbb91af972..452052bf9476 100644 --- a/drivers/staging/kpc2000/kpc2000_i2c.c +++ b/drivers/staging/kpc2000/kpc2000_i2c.c @@ -587,6 +587,9 @@ static int pi2c_probe(struct platform_device *pldev) priv->adapter.algo = _algorithm; res = platform_get_resource(pldev, IORESOURCE_MEM, 0); + if (!res) + return -ENXIO; + priv->smba = (unsigned long)ioremap_nocache(res->start, resource_size(res)); platform_set_drvdata(pldev, priv); -- 2.21.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 8/8] staging: kpc2000: kpc_i2c: Use devm_* API to manage mapped I/O space
The kpc_i2c driver does not unmap its I/O space upon error cases in the probe() function or upon remove(). Make the driver clean up after itself more maintainably by using the managed resource API. Signed-off-by: Geordan Neukum --- drivers/staging/kpc2000/kpc2000_i2c.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/staging/kpc2000/kpc2000_i2c.c b/drivers/staging/kpc2000/kpc2000_i2c.c index 51e91653e183..a434dd0b78c4 100644 --- a/drivers/staging/kpc2000/kpc2000_i2c.c +++ b/drivers/staging/kpc2000/kpc2000_i2c.c @@ -590,7 +590,9 @@ static int pi2c_probe(struct platform_device *pldev) if (!res) return -ENXIO; - priv->smba = (unsigned long)ioremap_nocache(res->start, resource_size(res)); + priv->smba = (unsigned long)devm_ioremap_nocache(>dev, +res->start, +resource_size(res)); if (!priv->smba) return -ENOMEM; -- 2.21.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/8] staging: kpc2000: kpc_i2c: Remove pldev from i2c_device structure
The i2c_device structure contains a member used to stash a pointer to
a platform_device. The driver contains no cases of this member being
used after initialization. Remove the unnecessary struct member and
the initialization of this member in the sole instance where the
driver creates a variable of type: struct i2c_device.
Signed-off-by: Geordan Neukum
---
drivers/staging/kpc2000/kpc2000_i2c.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/staging/kpc2000/kpc2000_i2c.c
b/drivers/staging/kpc2000/kpc2000_i2c.c
index 2c272ad8eca6..b2a9cda05f1b 100644
--- a/drivers/staging/kpc2000/kpc2000_i2c.c
+++ b/drivers/staging/kpc2000/kpc2000_i2c.c
@@ -36,7 +36,6 @@ MODULE_SOFTDEP("pre: i2c-dev");
struct i2c_device {
unsigned long smba;
struct i2c_adapter adapter;
- struct platform_device *pldev;
unsigned intfeatures;
};
@@ -595,7 +594,6 @@ static int pi2c_probe(struct platform_device *pldev)
res = platform_get_resource(pldev, IORESOURCE_MEM, 0);
priv->smba = (unsigned long)ioremap_nocache(res->start,
resource_size(res));
- priv->pldev = pldev;
pldev->dev.platform_data = priv;
priv->features |= FEATURE_IDF;
--
2.21.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 4/8] staging: kpc2000: kpc_i2c: Remove unnecessary consecutive newlines
The kpc2000_i2c.c file contains instances of unnecessary consecutive
newlines which impact the readability of the file. Remove these
unnecessary newlines.
Signed-off-by: Geordan Neukum
---
drivers/staging/kpc2000/kpc2000_i2c.c | 5 -
1 file changed, 5 deletions(-)
diff --git a/drivers/staging/kpc2000/kpc2000_i2c.c
b/drivers/staging/kpc2000/kpc2000_i2c.c
index 1d100bb7c548..1767f351a116 100644
--- a/drivers/staging/kpc2000/kpc2000_i2c.c
+++ b/drivers/staging/kpc2000/kpc2000_i2c.c
@@ -115,7 +115,6 @@ struct i2c_device {
#define PCI_DEVICE_ID_INTEL_LYNXPOINT_SMBUS 0x8c22
#define PCI_DEVICE_ID_INTEL_LYNXPOINT_LP_SMBUS 0x9c22
-
#define FEATURE_SMBUS_PEC BIT(0)
#define FEATURE_BLOCK_BUFFERBIT(1)
#define FEATURE_BLOCK_PROC BIT(2)
@@ -521,8 +520,6 @@ static s32 i801_access(struct i2c_adapter *adap, u16 addr,
unsigned short flags,
return 0;
}
-
-
static u32 i801_func(struct i2c_adapter *adapter)
{
struct i2c_device *priv = i2c_get_adapdata(adapter);
@@ -571,8 +568,6 @@ static const struct i2c_algorithm smbus_algorithm = {
.functionality = i801_func,
};
-
-
/
*** Part 2 - Driver Handlers ***
/
--
2.21.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/8] staging: kpc2000: kpc_i2c: Remove unused rw_sem
In pi2c_probe, a rw_sem is initialized and stashed off in the
i2c_device private runtime state struct. This rw_sem is never used
after initialization. Remove the rw_sem and cleanup unneeded header
inclusion.
Signed-off-by: Geordan Neukum
---
drivers/staging/kpc2000/kpc2000_i2c.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/staging/kpc2000/kpc2000_i2c.c
b/drivers/staging/kpc2000/kpc2000_i2c.c
index fb9a8386bcce..2c272ad8eca6 100644
--- a/drivers/staging/kpc2000/kpc2000_i2c.c
+++ b/drivers/staging/kpc2000/kpc2000_i2c.c
@@ -25,7 +25,6 @@
#include
#include
#include
-#include
#include
#include
#include "kpc.h"
@@ -38,7 +37,6 @@ struct i2c_device {
unsigned long smba;
struct i2c_adapter adapter;
struct platform_device *pldev;
- struct rw_semaphore rw_sem;
unsigned intfeatures;
};
@@ -606,7 +604,6 @@ static int pi2c_probe(struct platform_device *pldev)
priv->features |= FEATURE_BLOCK_BUFFER;
//init_MUTEX(>sem);
- init_rwsem(>rw_sem);
/* set up the sysfs linkage to our parent device */
priv->adapter.dev.parent = >dev;
--
2.21.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 5/8] staging: kpc2000: kpc_i2c: Use drvdata instead of platform_data
The kpc_i2c driver stashes private state data in the platform_data
member of its device structure. In general, the platform_data structure
is used for passing data to the driver during probe() rather than as a
storage area for runtime state data. Instead, use the drvdata member
for all state info meant to be accessible in driver callbacks.
Signed-off-by: Geordan Neukum
---
drivers/staging/kpc2000/kpc2000_i2c.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/kpc2000/kpc2000_i2c.c
b/drivers/staging/kpc2000/kpc2000_i2c.c
index 1767f351a116..e4bbb91af972 100644
--- a/drivers/staging/kpc2000/kpc2000_i2c.c
+++ b/drivers/staging/kpc2000/kpc2000_i2c.c
@@ -589,7 +589,7 @@ static int pi2c_probe(struct platform_device *pldev)
res = platform_get_resource(pldev, IORESOURCE_MEM, 0);
priv->smba = (unsigned long)ioremap_nocache(res->start,
resource_size(res));
- pldev->dev.platform_data = priv;
+ platform_set_drvdata(pldev, priv);
priv->features |= FEATURE_IDF;
priv->features |= FEATURE_I2C_BLOCK_READ;
@@ -620,7 +620,7 @@ static int pi2c_remove(struct platform_device *pldev)
{
struct i2c_device *lddev;
- lddev = (struct i2c_device *)pldev->dev.platform_data;
+ lddev = (struct i2c_device *)platform_get_drvdata(pldev);
i2c_del_adapter(>adapter);
--
2.21.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 3/8] staging: kpc2000: kpc_i2c: Use BIT macro rather than manual bit shifting
The FEATURES_* symbols use bit shifting of the style (1 << k) in order
to assign a certain meaning to the value of inividual bits being set
in the value of a given variable. Instead, use the BIT() macro in
order to improve readability and maintain consistency with the rest
of the kernel.
Signed-off-by: Geordan Neukum
---
drivers/staging/kpc2000/kpc2000_i2c.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/staging/kpc2000/kpc2000_i2c.c
b/drivers/staging/kpc2000/kpc2000_i2c.c
index b2a9cda05f1b..1d100bb7c548 100644
--- a/drivers/staging/kpc2000/kpc2000_i2c.c
+++ b/drivers/staging/kpc2000/kpc2000_i2c.c
@@ -116,12 +116,12 @@ struct i2c_device {
#define PCI_DEVICE_ID_INTEL_LYNXPOINT_LP_SMBUS 0x9c22
-#define FEATURE_SMBUS_PEC (1 << 0)
-#define FEATURE_BLOCK_BUFFER(1 << 1)
-#define FEATURE_BLOCK_PROC (1 << 2)
-#define FEATURE_I2C_BLOCK_READ (1 << 3)
+#define FEATURE_SMBUS_PEC BIT(0)
+#define FEATURE_BLOCK_BUFFERBIT(1)
+#define FEATURE_BLOCK_PROC BIT(2)
+#define FEATURE_I2C_BLOCK_READ BIT(3)
/* Not really a feature, but it's convenient to handle it as such */
-#define FEATURE_IDF (1 << 15)
+#define FEATURE_IDF BIT(15)
// FIXME!
#undef inb_p
--
2.21.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 0/8] staging: kpc2000: kpc_i2c: assorted driver cleanup
This series contains some patches aimed toward: - cleaning up unused/unneeded parts of driver state - a couple of small style fixups - a couple of API changes - better error handling in probe() in the kpc2000 i2c driver. Geordan Neukum (8): staging: kpc2000: kpc_i2c: Remove unused rw_sem staging: kpc2000: kpc_i2c: Remove pldev from i2c_device structure staging: kpc2000: kpc_i2c: Use BIT macro rather than manual bit shifting staging: kpc2000: kpc_i2c: Remove unnecessary consecutive newlines staging: kpc2000: kpc_i2c: Use drvdata instead of platform_data staging: kpc2000: kpc_i2c: fail probe if unable to get I/O resource staging: kpc2000: kpc_i2c: fail probe if unable to map I/O space staging: kpc2000: kpc_i2c: Use devm_* API to manage mapped I/O space drivers/staging/kpc2000/kpc2000_i2c.c | 33 --- 1 file changed, 15 insertions(+), 18 deletions(-) -- 2.21.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] staging: kpc2000: Add dependency on MFD_CORE to kconfig symbol 'KPC2000'
On Fri, May 24, 2019 at 2:38 AM Geordan Neukum wrote: > + depends on MFD_CORE In order for this to work in menuconfig, this either needs to be a select or I need to add a prompt to MFD_CORE. I don't have strong feelings either way, but all other Kconfig options which are related to the MFD_CORE appear to do a straight selection. Let me know what you think and I'll go that route. Thanks, Geordan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2] staging: kpc2000: Add dependency on MFD_CORE to kconfig symbol 'KPC2000'
The kpc2000 core makes calls against functions conditionally exported upon selection of the kconfig symbol MFD_CORE. Therefore, the kpc2000 core depends upon the mfd_core, and that dependency must be tracked in Kconfig to avoid potential build issues. Signed-off-by: Geordan Neukum --- v2 changes - base patch on staging-linus - only add MFD_CORE dependency as the UIO dependency has already been handled by YueHaibing drivers/staging/kpc2000/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/staging/kpc2000/Kconfig b/drivers/staging/kpc2000/Kconfig index febe4f8b30e5..ef0f4abe894a 100644 --- a/drivers/staging/kpc2000/Kconfig +++ b/drivers/staging/kpc2000/Kconfig @@ -2,6 +2,7 @@ config KPC2000 bool "Daktronics KPC Device support" + depends on MFD_CORE depends on PCI depends on UIO help -- 2.21.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/6] staging: kpc2000: make kconfig symbol 'KPC2000' select dependencies
On Wed, May 22, 2019 at 12:27 PM Greg Kroah-Hartman wrote: > depends on is better than select. There's a change to depend on UIO for > this code already in my -linus branch which will show up in Linus's tree > in a week or so. Noted on both accounts. Thanks for the feedback and sorry for the inconvenience on the latter. > Are you sure we need MFD_CORE as well for this code? I noticed the build issue when working locally. I was doing something along the lines of: 'make distclean && make x86_64_defconfig', selecting 'CONFIG_KPC2000' and 'CONFIG_UIO' via menuconfig, then running a good old 'make'. From make, I received an error along the lines of: ERROR: "mfd_remove_devices" [drivers/staging/kpc2000/kpc2000/kpc2000.ko] undefined! ERROR: "mfd_add_devices" [drivers/staging/kpc2000/kpc2000/kpc2000.ko] undefined! make[1]: *** [scripts/Makefile.modpost:91: __modpost] Error 1 make: *** [Makefile:1290: modules] Error 2 which appears to indicate that those two symbols are undefined. When I looked, it appeared that those symbols were exported from the mfd-core which is why I also threw in a select for that Kconfig symbol. Assuming that I didn't do something silly above, I'd be happy to submit a new patch (with only a depends on for MFD_CORE) as I continue trying to fix up the i2c driver. >Why hasn't that been seen on any build errors? To be honest, I can't say that I'm familiar with all of the different build bots out there so I can't even begin to speculate on that one. If someone could point me in the right direction there, I'd be happy to investigate further. Thanks again for your feedback all, Geordan Neukum ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/6] staging: kpc2000: kpc_i2c: remove unused module param disable_features
The module parameter 'disable_features' is currently unused. Therefore,
it should be removed.
Signed-off-by: Geordan Neukum
---
drivers/staging/kpc2000/kpc2000_i2c.c | 4
1 file changed, 4 deletions(-)
diff --git a/drivers/staging/kpc2000/kpc2000_i2c.c
b/drivers/staging/kpc2000/kpc2000_i2c.c
index 42061318d2d4..40a89998726e 100644
--- a/drivers/staging/kpc2000/kpc2000_i2c.c
+++ b/drivers/staging/kpc2000/kpc2000_i2c.c
@@ -126,10 +126,6 @@ struct i2c_device {
/* Not really a feature, but it's convenient to handle it as such */
#define FEATURE_IDF (1 << 15)
-static unsigned int disable_features;
-module_param(disable_features, uint, S_IRUGO | S_IWUSR);
-MODULE_PARM_DESC(disable_features, "Disable selected driver features");
-
// FIXME!
#undef inb_p
#define inb_p(a) readq((void*)a)
--
2.21.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 0/6] Minor updates to kpc_i2c driver and kpc2000 core
Attached are an assortment of minor updates to the kpc_i2c driver as well as a build fix for all of those who will need the KPC2000 core. Thanks, Geordan Geordan Neukum (6): staging: kpc2000: make kconfig symbol 'KPC2000' select dependencies staging: kpc2000: kpc_i2c: remove unused module param disable_features staging: kpc2000: kpc_i2c: newline fixups to meet linux style guide staging: kpc2000: kpc_i2c: use instead of staging: kpc2000: kpc_i2c: Remove unnecessary function tracing prints staging: kpc2000: kpc_i2c: add static qual to local symbols in kpc_i2c.c drivers/staging/kpc2000/Kconfig | 2 + drivers/staging/kpc2000/kpc2000_i2c.c | 118 +++--- 2 files changed, 34 insertions(+), 86 deletions(-) -- 2.21.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 5/6] staging: kpc2000: kpc_i2c: Remove unnecessary function tracing prints
Many of the functions in kpc_i2c log debug-level messages to the
kernel log message buffer upon invocation. This is unnecessary, as
debugging tools like kgdb, kdb, etc. or the tracing tool ftrace
should be able to provide this same information. Therefore, remove
these print statements.
Signed-off-by: Geordan Neukum
---
drivers/staging/kpc2000/kpc2000_i2c.c | 26 --
1 file changed, 26 deletions(-)
diff --git a/drivers/staging/kpc2000/kpc2000_i2c.c
b/drivers/staging/kpc2000/kpc2000_i2c.c
index 5d98ed54c05c..f9259c06b605 100644
--- a/drivers/staging/kpc2000/kpc2000_i2c.c
+++ b/drivers/staging/kpc2000/kpc2000_i2c.c
@@ -139,8 +139,6 @@ static int i801_check_pre(struct i2c_device *priv)
{
int status;
- dev_dbg(>adapter.dev, "%s\n", __func__);
-
status = inb_p(SMBHSTSTS(priv));
if (status & SMBHSTSTS_HOST_BUSY) {
dev_err(>adapter.dev, "SMBus is busy, can't use it!
(status=%x)\n", status);
@@ -165,8 +163,6 @@ static int i801_check_post(struct i2c_device *priv, int
status, int timeout)
{
int result = 0;
- dev_dbg(>adapter.dev, "%s\n", __func__);
-
/* If the SMBus is still busy, we give up */
if (timeout) {
dev_err(>adapter.dev, "Transaction timeout\n");
@@ -214,8 +210,6 @@ static int i801_transaction(struct i2c_device *priv, int
xact)
int result;
int timeout = 0;
- dev_dbg(>adapter.dev, "%s\n", __func__);
-
result = i801_check_pre(priv);
if (result < 0)
return result;
@@ -244,8 +238,6 @@ static void i801_wait_hwpec(struct i2c_device *priv)
int timeout = 0;
int status;
- dev_dbg(>adapter.dev, "%s\n", __func__);
-
do {
usleep_range(250, 500);
status = inb_p(SMBHSTSTS(priv));
@@ -262,8 +254,6 @@ static int i801_block_transaction_by_block(struct
i2c_device *priv, union i2c_sm
int i, len;
int status;
- dev_dbg(>adapter.dev, "%s\n", __func__);
-
inb_p(SMBHSTCNT(priv)); /* reset the data buffer index */
/* Use 32-byte buffer to process this transaction */
@@ -298,8 +288,6 @@ static int i801_block_transaction_byte_by_byte(struct
i2c_device *priv, union i2
int result;
int timeout;
- dev_dbg(>adapter.dev, "%s\n", __func__);
-
result = i801_check_pre(priv);
if (result < 0)
return result;
@@ -364,8 +352,6 @@ static int i801_block_transaction_byte_by_byte(struct
i2c_device *priv, union i2
static int i801_set_block_buffer_mode(struct i2c_device *priv)
{
- dev_dbg(>adapter.dev, "%s\n", __func__);
-
outb_p(inb_p(SMBAUXCTL(priv)) | SMBAUXCTL_E32B, SMBAUXCTL(priv));
if ((inb_p(SMBAUXCTL(priv)) & SMBAUXCTL_E32B) == 0)
return -EIO;
@@ -378,8 +364,6 @@ static int i801_block_transaction(struct i2c_device *priv,
union i2c_smbus_data
int result = 0;
//unsigned char hostc;
- dev_dbg(>adapter.dev, "%s\n", __func__);
-
if (command == I2C_SMBUS_I2C_BLOCK_DATA) {
if (read_write == I2C_SMBUS_WRITE) {
/* set I2C_EN bit in configuration register */
@@ -427,10 +411,6 @@ static s32 i801_access(struct i2c_adapter *adap, u16 addr,
unsigned short flags,
int ret, xact = 0;
struct i2c_device *priv = i2c_get_adapdata(adap);
- dev_dbg(>adapter.dev,
- "%s (addr=%0d) flags=%x read_write=%x command=%x size=%x",
- __func__, addr, flags, read_write, command, size);
-
hwpec = (priv->features & FEATURE_SMBUS_PEC) && (flags &
I2C_CLIENT_PEC) && size != I2C_SMBUS_QUICK && size != I2C_SMBUS_I2C_BLOCK_DATA;
switch (size) {
@@ -605,9 +585,6 @@ int pi2c_probe(struct platform_device *pldev)
struct i2c_device *priv;
struct resource *res;
- dev_dbg(>dev, "%s(pldev = %p '%s')\n", __func__, pldev,
- pldev->name);
-
priv = devm_kzalloc(>dev, sizeof(*priv), GFP_KERNEL);
if (!priv)
return -ENOMEM;
@@ -653,9 +630,6 @@ int pi2c_remove(struct platform_device *pldev)
{
struct i2c_device *lddev;
- dev_dbg(>dev, "%s(pldev = %p '%s')\n", __func__, pldev,
- pldev->name);
-
lddev = (struct i2c_device *)pldev->dev.platform_data;
i2c_del_adapter(>adapter);
--
2.21.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 4/6] staging: kpc2000: kpc_i2c: use instead of
Rather than include asm/io.h, include linux/io.h. Issue reported by the script checkpatch.pl. Signed-off-by: Geordan Neukum --- drivers/staging/kpc2000/kpc2000_i2c.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/kpc2000/kpc2000_i2c.c b/drivers/staging/kpc2000/kpc2000_i2c.c index a1ebc2386d70..5d98ed54c05c 100644 --- a/drivers/staging/kpc2000/kpc2000_i2c.c +++ b/drivers/staging/kpc2000/kpc2000_i2c.c @@ -19,7 +19,7 @@ #include #include #include -#include +#include #include #include #include -- 2.21.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 6/6] staging: kpc2000: kpc_i2c: add static qual to local symbols in kpc_i2c.c
kpc_i2c.c declares:
- two functions
- pi2c_probe()
- pi2c_remove()
- one struct
- i2c_plat_driver_i
which are local to the file, yet missing the static qualifier. Add the
static qualifier to these symbols.
Signed-off-by: Geordan Neukum
---
drivers/staging/kpc2000/kpc2000_i2c.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/kpc2000/kpc2000_i2c.c
b/drivers/staging/kpc2000/kpc2000_i2c.c
index f9259c06b605..97e738349ba2 100644
--- a/drivers/staging/kpc2000/kpc2000_i2c.c
+++ b/drivers/staging/kpc2000/kpc2000_i2c.c
@@ -579,7 +579,7 @@ static const struct i2c_algorithm smbus_algorithm = {
/
*** Part 2 - Driver Handlers ***
/
-int pi2c_probe(struct platform_device *pldev)
+static int pi2c_probe(struct platform_device *pldev)
{
int err;
struct i2c_device *priv;
@@ -626,7 +626,7 @@ int pi2c_probe(struct platform_device *pldev)
return 0;
}
-int pi2c_remove(struct platform_device *pldev)
+static int pi2c_remove(struct platform_device *pldev)
{
struct i2c_device *lddev;
@@ -644,7 +644,7 @@ int pi2c_remove(struct platform_device *pldev)
return 0;
}
-struct platform_driver i2c_plat_driver_i = {
+static struct platform_driver i2c_plat_driver_i = {
.probe = pi2c_probe,
.remove = pi2c_remove,
.driver = {
--
2.21.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 3/6] staging: kpc2000: kpc_i2c: newline fixups to meet linux style guide
The linux coding style document states:
1) That braces should not be used where a single single statement
will do. Therefore all instances of single block statements
wrapped in braces that do not meet the qualifications of any
of the exceptions to the rule should be fixed up.
2) That the declaration of variables local to a given function
should be immediately followed by a blank newline. Therefore,
the single instance of this in kpc2000_i2c.c should be fixed
up.
Signed-off-by: Geordan Neukum
---
drivers/staging/kpc2000/kpc2000_i2c.c | 82 ++-
1 file changed, 29 insertions(+), 53 deletions(-)
diff --git a/drivers/staging/kpc2000/kpc2000_i2c.c
b/drivers/staging/kpc2000/kpc2000_i2c.c
index 40a89998726e..a1ebc2386d70 100644
--- a/drivers/staging/kpc2000/kpc2000_i2c.c
+++ b/drivers/staging/kpc2000/kpc2000_i2c.c
@@ -178,9 +178,8 @@ static int i801_check_post(struct i2c_device *priv, int
status, int timeout)
/* Check if it worked */
status = inb_p(SMBHSTSTS(priv));
- if ((status & SMBHSTSTS_HOST_BUSY) || !(status &
SMBHSTSTS_FAILED)) {
+ if ((status & SMBHSTSTS_HOST_BUSY) || !(status &
SMBHSTSTS_FAILED))
dev_err(>adapter.dev, "Failed terminating the
transaction\n");
- }
outb_p(STATUS_FLAGS, SMBHSTSTS(priv));
return -ETIMEDOUT;
}
@@ -202,9 +201,8 @@ static int i801_check_post(struct i2c_device *priv, int
status, int timeout)
/* Clear error flags */
outb_p(status & STATUS_FLAGS, SMBHSTSTS(priv));
status = inb_p(SMBHSTSTS(priv)) & STATUS_FLAGS;
- if (status) {
+ if (status)
dev_warn(>adapter.dev, "Failed clearing status
flags at end of transaction (%02x)\n", status);
- }
}
return result;
@@ -219,9 +217,8 @@ static int i801_transaction(struct i2c_device *priv, int
xact)
dev_dbg(>adapter.dev, "%s\n", __func__);
result = i801_check_pre(priv);
- if (result < 0) {
+ if (result < 0)
return result;
- }
/* the current contents of SMBHSTCNT can be overwritten, since PEC,
* INTREN, SMBSCMD are passed in xact
*/
@@ -234,9 +231,8 @@ static int i801_transaction(struct i2c_device *priv, int
xact)
} while ((status & SMBHSTSTS_HOST_BUSY) && (timeout++ < MAX_RETRIES));
result = i801_check_post(priv, status, timeout > MAX_RETRIES);
- if (result < 0) {
+ if (result < 0)
return result;
- }
outb_p(SMBHSTSTS_INTR, SMBHSTSTS(priv));
return 0;
@@ -255,9 +251,8 @@ static void i801_wait_hwpec(struct i2c_device *priv)
status = inb_p(SMBHSTSTS(priv));
} while ((!(status & SMBHSTSTS_INTR)) && (timeout++ < MAX_RETRIES));
- if (timeout > MAX_RETRIES) {
+ if (timeout > MAX_RETRIES)
dev_dbg(>adapter.dev, "PEC Timeout!\n");
- }
outb_p(status, SMBHSTSTS(priv));
}
@@ -275,26 +270,22 @@ static int i801_block_transaction_by_block(struct
i2c_device *priv, union i2c_sm
if (read_write == I2C_SMBUS_WRITE) {
len = data->block[0];
outb_p(len, SMBHSTDAT0(priv));
- for (i = 0; i < len; i++) {
+ for (i = 0; i < len; i++)
outb_p(data->block[i+1], SMBBLKDAT(priv));
- }
}
status = i801_transaction(priv, I801_BLOCK_DATA | ENABLE_INT9 |
I801_PEC_EN * hwpec);
- if (status) {
+ if (status)
return status;
- }
if (read_write == I2C_SMBUS_READ) {
len = inb_p(SMBHSTDAT0(priv));
- if (len < 1 || len > I2C_SMBUS_BLOCK_MAX) {
+ if (len < 1 || len > I2C_SMBUS_BLOCK_MAX)
return -EPROTO;
- }
data->block[0] = len;
- for (i = 0; i < len; i++) {
+ for (i = 0; i < len; i++)
data->block[i + 1] = inb_p(SMBBLKDAT(priv));
- }
}
return 0;
}
@@ -310,9 +301,8 @@ static int i801_block_transaction_byte_by_byte(struct
i2c_device *priv, union i2
dev_dbg(>adapter.dev, "%s\n", __func__);
result = i801_check_pre(priv);
- if (result < 0) {
+ if (result < 0)
return result;
- }
len = data->block[0];
@@ -323,23 +313,20 @@ static int i801_block_transaction_byte_by_byte(struct
i2c_device *priv, union i2
for (i = 1; i <= len; i++) {
if (i == len && read_write == I2C_SMBUS_READ) {
- if (
[PATCH 1/6] staging: kpc2000: make kconfig symbol 'KPC2000' select dependencies
The kpc2000 core makes calls against functions which are conditionally exported upon the kconfig symbols 'MFD_CORE' and 'UIO' being selected Therefore, in order to guarantee correct compilation, the 'KPC2000' kconfig symbol (which brings in that code) must explicitly select its hard dependencies. Signed-off-by: Geordan Neukum --- drivers/staging/kpc2000/Kconfig | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/staging/kpc2000/Kconfig b/drivers/staging/kpc2000/Kconfig index fb5922928f47..8992dc67ff37 100644 --- a/drivers/staging/kpc2000/Kconfig +++ b/drivers/staging/kpc2000/Kconfig @@ -3,6 +3,8 @@ config KPC2000 bool "Daktronics KPC Device support" depends on PCI + select MFD_CORE + select UIO help Select this if you wish to use the Daktronics KPC PCI devices -- 2.21.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 0/5] Updates to staging driver: kpc_i2c
> On Mon, May 20, 2019 at 10:30:26AM +0200, Greg Kroah-Hartman wrote: > > All now queued up. I'll rebase my patches that move this file on top of > yours, as kbuild found some problems with mine, and I'll resend them to > the list. > > Thanks. ** Same content as last reply, just realized that I had configured my ** email client to do something wrong. Resend for readability's sake. Additionally, I plan on trying to clean up that driver a bit more. Should I base my future patches off of the staging tree so that I'll have the "latest" driver as my basepoint? I don't want to cause any headaches for anyone in the future. Apologies, if I missed something obvious on the newbies wiki. Assuming that I did not, I will certainly go ahead and try to document this case either on or as a link from the "sending your first patch" page. Cheers, Geordan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 0/5] Updates to staging driver: kpc_i2c
On Mon, May 20, 2019 at 10:30:26AM +0200, Greg Kroah-Hartman wrote: > On Sat, May 18, 2019 at 02:29:55AM +0000, Geordan Neukum wrote: > > Attached are an assortment of updates to the kpc_i2c driver in the > > staging subtree. > > All now queued up. I'll rebase my patches that move this file on top of > yours, as kbuild found some problems with mine, and I'll resend them to > the list. > Thanks. Additionally, I plan on trying to clean up that driver a bit more. Should I base my future patches off of the staging tree so that I'll have the "latest" driver as my basepoint? I don't want to cause any headaches for anyone in the future. Apologies, if I missed something obvious on the newbies wiki. Assuming that I did not, I will certainly go ahead and try to document this case either on or as a link from the "sending your first patch" page. Cheers, Geordan On Mon, May 20, 2019 at 8:30 AM Greg Kroah-Hartman wrote: > > On Sat, May 18, 2019 at 02:29:55AM +, Geordan Neukum wrote: > > Attached are an assortment of updates to the kpc_i2c driver in the > > staging subtree. > > All now queued up. I'll rebase my patches that move this file on top of > yours, as kbuild found some problems with mine, and I'll resend them to > the list. > > thanks, > > greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 4/5] staging: kpc2000: kpc_i2c: use %s with __func__ identifier in log messages
On Fri, May 17, 2019 at 07:58:19PM -0700, Joe Perches wrote:
> On Sat, 2019-05-18 at 02:29 +0000, Geordan Neukum wrote:
> > Throughout i2c_driver.c, there are instances where the log strings
> > contain the function's name hardcoded into the string. Instead, use the
> > printk conversion specifier '%s' with the __func__ preprocessor
> > identifier to more maintainably print the function's name.
>
> Might as well remove all of these and use the
> builtin ftrace function tracing mechanism instead.
>
> > diff --git a/drivers/staging/kpc2000/kpc_i2c/i2c_driver.c
> > b/drivers/staging/kpc2000/kpc_i2c/i2c_driver.c
> []
> > @@ -142,7 +142,7 @@ static int i801_check_pre(struct i2c_device *priv)
> > {
> > int status;
> >
> > - dev_dbg(>adapter.dev, "i801_check_pre\n");
> > + dev_dbg(>adapter.dev, "%s\n", __func__);
>
> etc...
>
Joe/All,
Acknowledged. I apologize for the inconvenience there -- I was
unfamiliar with that API until receiving your email. I'll hold on
additional uploads until other reviewers have had time to take a
look, but I do plan on leveraging the ftrace API instead of just
using __func__ and %s in my printk strings in the upcoming 'v2'
patchset.
Thanks for your feedback,
Geordan
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 4/5] staging: kpc2000: kpc_i2c: use %s with __func__ identifier in log messages
Throughout i2c_driver.c, there are instances where the log strings
contain the function's name hardcoded into the string. Instead, use the
printk conversion specifier '%s' with the __func__ preprocessor
identifier to more maintainably print the function's name.
Signed-off-by: Geordan Neukum
---
drivers/staging/kpc2000/kpc_i2c/i2c_driver.c | 27 +++-
1 file changed, 15 insertions(+), 12 deletions(-)
diff --git a/drivers/staging/kpc2000/kpc_i2c/i2c_driver.c
b/drivers/staging/kpc2000/kpc_i2c/i2c_driver.c
index 9b9de81c8548..03e401322a18 100644
--- a/drivers/staging/kpc2000/kpc_i2c/i2c_driver.c
+++ b/drivers/staging/kpc2000/kpc_i2c/i2c_driver.c
@@ -142,7 +142,7 @@ static int i801_check_pre(struct i2c_device *priv)
{
int status;
- dev_dbg(>adapter.dev, "i801_check_pre\n");
+ dev_dbg(>adapter.dev, "%s\n", __func__);
status = inb_p(SMBHSTSTS(priv));
if (status & SMBHSTSTS_HOST_BUSY) {
@@ -168,7 +168,7 @@ static int i801_check_post(struct i2c_device *priv, int
status, int timeout)
{
int result = 0;
- dev_dbg(>adapter.dev, "i801_check_post\n");
+ dev_dbg(>adapter.dev, "%s\n", __func__);
/* If the SMBus is still busy, we give up */
if (timeout) {
@@ -219,7 +219,7 @@ static int i801_transaction(struct i2c_device *priv, int
xact)
int result;
int timeout = 0;
- dev_dbg(>adapter.dev, "i801_transaction\n");
+ dev_dbg(>adapter.dev, "%s\n", __func__);
result = i801_check_pre(priv);
if (result < 0) {
@@ -250,7 +250,7 @@ static void i801_wait_hwpec(struct i2c_device *priv)
int timeout = 0;
int status;
- dev_dbg(>adapter.dev, "i801_wait_hwpec\n");
+ dev_dbg(>adapter.dev, "%s\n", __func__);
do {
usleep_range(250, 500);
@@ -269,7 +269,7 @@ static int i801_block_transaction_by_block(struct
i2c_device *priv, union i2c_sm
int i, len;
int status;
- dev_dbg(>adapter.dev, "i801_block_transaction_by_block\n");
+ dev_dbg(>adapter.dev, "%s\n", __func__);
inb_p(SMBHSTCNT(priv)); /* reset the data buffer index */
@@ -309,7 +309,7 @@ static int i801_block_transaction_byte_by_byte(struct
i2c_device *priv, union i2
int result;
int timeout;
- dev_dbg(>adapter.dev, "i801_block_transaction_byte_by_byte\n");
+ dev_dbg(>adapter.dev, "%s\n", __func__);
result = i801_check_pre(priv);
if (result < 0) {
@@ -383,7 +383,7 @@ static int i801_block_transaction_byte_by_byte(struct
i2c_device *priv, union i2
static int i801_set_block_buffer_mode(struct i2c_device *priv)
{
- dev_dbg(>adapter.dev, "i801_set_block_buffer_mode\n");
+ dev_dbg(>adapter.dev, "%s\n", __func__);
outb_p(inb_p(SMBAUXCTL(priv)) | SMBAUXCTL_E32B, SMBAUXCTL(priv));
if ((inb_p(SMBAUXCTL(priv)) & SMBAUXCTL_E32B) == 0) {
@@ -398,7 +398,7 @@ static int i801_block_transaction(struct i2c_device *priv,
union i2c_smbus_data
int result = 0;
//unsigned char hostc;
- dev_dbg(>adapter.dev, "i801_block_transaction\n");
+ dev_dbg(>adapter.dev, "%s\n", __func__);
if (command == I2C_SMBUS_I2C_BLOCK_DATA) {
if (read_write == I2C_SMBUS_WRITE) {
@@ -450,8 +450,9 @@ static s32 i801_access(struct i2c_adapter *adap, u16 addr,
unsigned short flags,
int ret, xact = 0;
struct i2c_device *priv = i2c_get_adapdata(adap);
- dev_dbg(>adapter.dev, "i801_access (addr=%0d) flags=%x
read_write=%x command=%x size=%x",
- addr, flags, read_write, command, size );
+ dev_dbg(>adapter.dev,
+ "%s (addr=%0d) flags=%x read_write=%x command=%x size=%x",
+ __func__, addr, flags, read_write, command, size);
hwpec = (priv->features & FEATURE_SMBUS_PEC) && (flags &
I2C_CLIENT_PEC) && size != I2C_SMBUS_QUICK && size != I2C_SMBUS_I2C_BLOCK_DATA;
@@ -626,7 +627,8 @@ int pi2c_probe(struct platform_device *pldev)
struct i2c_device *priv;
struct resource *res;
- dev_dbg(>dev, "pi2c_probe(pldev = %p '%s')\n", pldev,
pldev->name);
+ dev_dbg(>dev, "%s(pldev = %p '%s')\n", __func__, pldev,
+ pldev->name);
priv = devm_kzalloc(>dev, sizeof(*priv), GFP_KERNEL);
if (!priv) {
@@ -673,7 +675,8 @@ int pi2c_probe(struct platform_device *pldev)
int pi2c_remove(struct platform_device *pldev)
{
struct i2c_device *lddev;
- dev_dbg(>dev, "pi2c_remove(pldev = %p '%s')\n", pldev,
pldev->name);
+ dev_dbg(>dev, "%s(pldev = %p '%s')\n", __func__, pldev,
+ pldev->name);
lddev = (struct i2c_device *)pldev->dev.platform_data;
--
2.21.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/5] staging: kpc2000: kpc_i2c: reindent i2c_driver.c
i2c_driver.c uses a mixture of space and tab indentations which
conflicts with the kernel coding style guide. Reindent i2c_driver.c.
Signed-off-by: Geordan Neukum
---
drivers/staging/kpc2000/kpc_i2c/i2c_driver.c | 1014 +-
1 file changed, 507 insertions(+), 507 deletions(-)
diff --git a/drivers/staging/kpc2000/kpc_i2c/i2c_driver.c
b/drivers/staging/kpc2000/kpc_i2c/i2c_driver.c
index 0fb068b2408d..6dda4eb6de75 100644
--- a/drivers/staging/kpc2000/kpc_i2c/i2c_driver.c
+++ b/drivers/staging/kpc2000/kpc_i2c/i2c_driver.c
@@ -1,14 +1,14 @@
// SPDX-License-Identifier: GPL-2.0+
/* Copyright (c) 2014-2018 Daktronics,
- Matt Sickler ,
- Jordon Hofer
+ Matt Sickler ,
+ Jordon Hofer
Adapted i2c-i801.c for use with Kadoka hardware.
Copyright (c) 1998 - 2002 Frodo Looijaard ,
Philip Edelbrock , and Mark D. Studebaker
Copyright (C) 2007 - 2012 Jean Delvare
Copyright (C) 2010 Intel Corporation,
- David Woodhouse
+ David Woodhouse
*/
#include
#include
@@ -29,11 +29,11 @@ MODULE_AUTHOR("matt.sick...@daktronics.com");
MODULE_SOFTDEP("pre: i2c-dev");
struct i2c_device {
-unsigned long smba;
-struct i2c_adapter adapter;
-struct platform_device *pldev;
-struct rw_semaphore rw_sem;
-unsigned intfeatures;
+ unsigned long smba;
+ struct i2c_adapter adapter;
+ struct platform_device *pldev;
+ struct rw_semaphore rw_sem;
+ unsigned intfeatures;
};
/*
@@ -134,479 +134,479 @@ MODULE_PARM_DESC(disable_features, "Disable selected
driver features");
Return 0 if it is, -EBUSY if it is not. */
static int i801_check_pre(struct i2c_device *priv)
{
-int status;
-
-dev_dbg(>adapter.dev, "i801_check_pre\n");
-
-status = inb_p(SMBHSTSTS(priv));
-if (status & SMBHSTSTS_HOST_BUSY) {
-dev_err(>adapter.dev, "SMBus is busy, can't use it!
(status=%x)\n", status);
-return -EBUSY;
-}
-
-status &= STATUS_FLAGS;
-if (status) {
-//dev_dbg(>adapter.dev, "Clearing status flags (%02x)\n",
status);
-outb_p(status, SMBHSTSTS(priv));
-status = inb_p(SMBHSTSTS(priv)) & STATUS_FLAGS;
-if (status) {
- dev_err(>adapter.dev, "Failed clearing status flags (%02x)\n",
status);
- return -EBUSY;
-}
-}
-return 0;
+ int status;
+
+ dev_dbg(>adapter.dev, "i801_check_pre\n");
+
+ status = inb_p(SMBHSTSTS(priv));
+ if (status & SMBHSTSTS_HOST_BUSY) {
+ dev_err(>adapter.dev, "SMBus is busy, can't use it!
(status=%x)\n", status);
+ return -EBUSY;
+ }
+
+ status &= STATUS_FLAGS;
+ if (status) {
+ //dev_dbg(>adapter.dev, "Clearing status flags (%02x)\n",
status);
+ outb_p(status, SMBHSTSTS(priv));
+ status = inb_p(SMBHSTSTS(priv)) & STATUS_FLAGS;
+ if (status) {
+ dev_err(>adapter.dev, "Failed clearing status
flags (%02x)\n", status);
+ return -EBUSY;
+ }
+ }
+ return 0;
}
/* Convert the status register to an error code, and clear it. */
static int i801_check_post(struct i2c_device *priv, int status, int timeout)
{
-int result = 0;
-
-dev_dbg(>adapter.dev, "i801_check_post\n");
-
-/* If the SMBus is still busy, we give up */
-if (timeout) {
-dev_err(>adapter.dev, "Transaction timeout\n");
-/* try to stop the current command */
-dev_dbg(>adapter.dev, "Terminating the current operation\n");
-outb_p(inb_p(SMBHSTCNT(priv)) | SMBHSTCNT_KILL, SMBHSTCNT(priv));
-usleep_range(1000, 2000);
-outb_p(inb_p(SMBHSTCNT(priv)) & (~SMBHSTCNT_KILL), SMBHSTCNT(priv));
-
-/* Check if it worked */
-status = inb_p(SMBHSTSTS(priv));
-if ((status & SMBHSTSTS_HOST_BUSY) || !(status & SMBHSTSTS_FAILED)) {
-dev_err(>adapter.dev, "Failed terminating the
transaction\n");
-}
-outb_p(STATUS_FLAGS, SMBHSTSTS(priv));
-return -ETIMEDOUT;
-}
-
-if (status & SMBHSTSTS_FAILED) {
-result = -EIO;
-dev_err(>adapter.dev, "Transaction failed\n");
-}
-if (status & SMBHSTSTS_DEV_ERR) {
-result = -ENXIO;
-dev_dbg(>adapter.dev, "No response\n");
-}
-if (status & SMBHSTSTS_BUS_ERR) {
-result = -EAGAIN;
-dev_dbg(>adapter.dev, "
[PATCH 5/5] staging: kpc2000: kpc_i2c: fixup block comment style in i2c_driver.c
Throughout i2c_driver.c, there are numerous deviations from the two
standards of:
- placing a '*' at the beginning of every line containing a
block comment.
- placing the closing comment marker '*/' on a new line.
Instead, use a block comment style that is more consistent with the
prescribed guidelines.
Signed-off-by: Geordan Neukum
---
drivers/staging/kpc2000/kpc_i2c/i2c_driver.c | 36
1 file changed, 21 insertions(+), 15 deletions(-)
diff --git a/drivers/staging/kpc2000/kpc_i2c/i2c_driver.c
b/drivers/staging/kpc2000/kpc_i2c/i2c_driver.c
index 03e401322a18..986148c72185 100644
--- a/drivers/staging/kpc2000/kpc_i2c/i2c_driver.c
+++ b/drivers/staging/kpc2000/kpc_i2c/i2c_driver.c
@@ -137,7 +137,8 @@ MODULE_PARM_DESC(disable_features, "Disable selected driver
features");
#define outb_p(d,a) writeq(d,(void*)a)
/* Make sure the SMBus host is ready to start transmitting.
- Return 0 if it is, -EBUSY if it is not. */
+ * Return 0 if it is, -EBUSY if it is not.
+ */
static int i801_check_pre(struct i2c_device *priv)
{
int status;
@@ -226,7 +227,8 @@ static int i801_transaction(struct i2c_device *priv, int
xact)
return result;
}
/* the current contents of SMBHSTCNT can be overwritten, since PEC,
-* INTREN, SMBSCMD are passed in xact */
+* INTREN, SMBSCMD are passed in xact
+*/
outb_p(xact | I801_START, SMBHSTCNT(priv));
/* We will always wait for a fraction of a second! */
@@ -424,8 +426,9 @@ static int i801_block_transaction(struct i2c_device *priv,
union i2c_smbus_data
}
/* Experience has shown that the block buffer can only be used for
- SMBus (not I2C) block transactions, even though the datasheet
- doesn't mention this limitation. */
+* SMBus (not I2C) block transactions, even though the datasheet
+* doesn't mention this limitation.
+*/
if ((priv->features & FEATURE_BLOCK_BUFFER) && command !=
I2C_SMBUS_I2C_BLOCK_DATA && i801_set_block_buffer_mode(priv) == 0) {
result = i801_block_transaction_by_block(priv, data,
read_write, hwpec);
} else {
@@ -499,11 +502,13 @@ static s32 i801_access(struct i2c_adapter *adap, u16
addr, unsigned short flags,
case I2C_SMBUS_I2C_BLOCK_DATA:
dev_dbg(>adapter.dev, " [acc] SMBUS_I2C_BLOCK_DATA\n");
/* NB: page 240 of ICH5 datasheet shows that the R/#W
-* bit should be cleared here, even when reading */
+* bit should be cleared here, even when reading
+*/
outb_p((addr & 0x7f) << 1, SMBHSTADD(priv));
if (read_write == I2C_SMBUS_READ) {
/* NB: page 240 of ICH5 datasheet also shows
-* that DATA1 is the cmd field when reading */
+* that DATA1 is the cmd field when reading
+*/
outb_p(command, SMBHSTDAT1(priv));
} else {
outb_p(command, SMBHSTCMD(priv));
@@ -533,8 +538,9 @@ static s32 i801_access(struct i2c_adapter *adap, u16 addr,
unsigned short flags,
}
/* Some BIOSes don't like it when PEC is enabled at reboot or resume
- time, so we forcibly disable it after every transaction. Turn off
- E32B for the same reason. */
+* time, so we forcibly disable it after every transaction. Turn off
+* E32B for the same reason.
+*/
if (hwpec || block) {
dev_dbg(>adapter.dev, " [acc] hwpec || block\n");
outb_p(inb_p(SMBAUXCTL(priv)) & ~(SMBAUXCTL_CRC |
SMBAUXCTL_E32B), SMBAUXCTL(priv));
@@ -573,13 +579,13 @@ static u32 i801_func(struct i2c_adapter *adapter)
struct i2c_device *priv = i2c_get_adapdata(adapter);
/* original settings
- u32 f = I2C_FUNC_SMBUS_QUICK | I2C_FUNC_SMBUS_BYTE |
- I2C_FUNC_SMBUS_BYTE_DATA | I2C_FUNC_SMBUS_WORD_DATA |
- I2C_FUNC_SMBUS_BLOCK_DATA | I2C_FUNC_SMBUS_WRITE_I2C_BLOCK |
- ((priv->features & FEATURE_SMBUS_PEC) ? I2C_FUNC_SMBUS_PEC : 0) |
- ((priv->features & FEATURE_I2C_BLOCK_READ) ?
- I2C_FUNC_SMBUS_READ_I2C_BLOCK : 0);
- */
+* u32 f = I2C_FUNC_SMBUS_QUICK | I2C_FUNC_SMBUS_BYTE |
+* I2C_FUNC_SMBUS_BYTE_DATA | I2C_FUNC_SMBUS_WORD_DATA |
+* I2C_FUNC_SMBUS_BLOCK_DATA | I2C_FUNC_SMBUS_WRITE_I2C_BLOCK |
+* ((priv->features & FEATURE_SMBUS_PEC) ? I2C_FUNC_SMBUS_PEC : 0) |
+* ((priv->features & FEATURE_I2C_BLOCK_READ) ?
+* I2C_FUNC_SMBUS_READ_I2C_BLOCK : 0);
+*/
// http://lxr.free-electrons.com/source/include/uapi/linux/i2c.h#L85
--
2.21.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 0/5] Updates to staging driver: kpc_i2c
Attached are an assortment of updates to the kpc_i2c driver in the staging subtree. As a quick summary: Patches 1, 4, and 5 address style concerns raised by the checkpatch script. Patch 1 (a reindentation fix) likely added additional 'line length' warnings, but given the fact that they were only hidden due to a nonstandard indentation style, I elected to defer that work to a future patchset. If the reviewers should disagree with that choice, I'd be happy to fix up those issues in a v2 upload. Patch 2 is a reformatting / reorganization of the copyright header at the top of the file. I attempted to look at some other drivers in the i2c subsystem for inspiration, but I'd be happy to drop this isn't as simple an update as it seems. Patch 3 addresses a potential bug in the cleanup of memory allocated in the probe method. Geordan Neukum (5): staging: kpc2000: kpc_i2c: reindent i2c_driver.c staging: kpc2000: kpc_i2c: reformat copyright for better readability staging: kpc2000: kpc_i2c: prevent memory leak in probe() error case staging: kpc2000: kpc_i2c: use %s with __func__ identifier in log messages staging: kpc2000: kpc_i2c: fixup block comment style in i2c_driver.c drivers/staging/kpc2000/kpc_i2c/i2c_driver.c | 1043 +- 1 file changed, 527 insertions(+), 516 deletions(-) -- 2.21.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/5] staging: kpc2000: kpc_i2c: reformat copyright for better readability
The copyright header in i2c_driver.c is difficult to read and not chronologically ordered. Reformat and reorganize the copyright header to be similar to other drivers in the i2c subsystem. Signed-off-by: Geordan Neukum --- drivers/staging/kpc2000/kpc_i2c/i2c_driver.c | 30 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/drivers/staging/kpc2000/kpc_i2c/i2c_driver.c b/drivers/staging/kpc2000/kpc_i2c/i2c_driver.c index 6dda4eb6de75..6cb63d20b00f 100644 --- a/drivers/staging/kpc2000/kpc_i2c/i2c_driver.c +++ b/drivers/staging/kpc2000/kpc_i2c/i2c_driver.c @@ -1,15 +1,21 @@ // SPDX-License-Identifier: GPL-2.0+ -/* Copyright (c) 2014-2018 Daktronics, - Matt Sickler , - Jordon Hofer -Adapted i2c-i801.c for use with Kadoka hardware. -Copyright (c) 1998 - 2002 Frodo Looijaard , -Philip Edelbrock , and Mark D. Studebaker - -Copyright (C) 2007 - 2012 Jean Delvare -Copyright (C) 2010 Intel Corporation, - David Woodhouse -*/ +/* + * KPC2000 i2c driver + * + * Adapted i2c-i801.c for use with Kadoka hardware. + * + * Copyright (C) 1998 - 2002 + * Frodo Looijaard , + * Philip Edelbrock , + * Mark D. Studebaker + * Copyright (C) 2007 - 2012 + * Jean Delvare + * Copyright (C) 2010 Intel Corporation + * David Woodhouse + * Copyright (C) 2014-2018 Daktronics + * Matt Sickler , + * Jordon Hofer + */ #include #include #include @@ -445,7 +451,7 @@ static s32 i801_access(struct i2c_adapter *adap, u16 addr, unsigned short flags, struct i2c_device *priv = i2c_get_adapdata(adap); dev_dbg(>adapter.dev, "i801_access (addr=%0d) flags=%x read_write=%x command=%x size=%x", - addr, flags, read_write, command, size ); + addr, flags, read_write, command, size ); hwpec = (priv->features & FEATURE_SMBUS_PEC) && (flags & I2C_CLIENT_PEC) && size != I2C_SMBUS_QUICK && size != I2C_SMBUS_I2C_BLOCK_DATA; -- 2.21.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 3/5] staging: kpc2000: kpc_i2c: prevent memory leak in probe() error case
The probe() function performs a kzalloc to dynamically allocate memory
at runtime. If the allocation succeeds, yet invoking the function
i2c_add_adapter fails, the dynamically allocated memory is never freed.
Change the allocation to use the managed allocation API instead and
remove the manual freeing of the memory in the remove() function.
Signed-off-by: Geordan Neukum
---
drivers/staging/kpc2000/kpc_i2c/i2c_driver.c | 6 +-
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/staging/kpc2000/kpc_i2c/i2c_driver.c
b/drivers/staging/kpc2000/kpc_i2c/i2c_driver.c
index 6cb63d20b00f..9b9de81c8548 100644
--- a/drivers/staging/kpc2000/kpc_i2c/i2c_driver.c
+++ b/drivers/staging/kpc2000/kpc_i2c/i2c_driver.c
@@ -628,7 +628,7 @@ int pi2c_probe(struct platform_device *pldev)
dev_dbg(>dev, "pi2c_probe(pldev = %p '%s')\n", pldev,
pldev->name);
- priv = kzalloc(sizeof(struct i2c_device), GFP_KERNEL);
+ priv = devm_kzalloc(>dev, sizeof(*priv), GFP_KERNEL);
if (!priv) {
return -ENOMEM;
}
@@ -685,10 +685,6 @@ int pi2c_remove(struct platform_device *pldev)
//pci_set_drvdata(dev, NULL);
//cdev_del(>cdev);
- if(lddev != 0) {
- kfree(lddev);
- pldev->dev.platform_data = 0;
- }
return 0;
}
--
2.21.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
