Re: [PATCH] [RFC] i2c: imx: make use of format specifier %dE
On Thu, Aug 29, 2019 at 06:29:05AM +0200, Uwe Kleine-König wrote: > I created a patch that teaches printk et al to emit a symbolic error > name for an error valued integer[1]. With that applied > > dev_err(&pdev->dev, "can't enable I2C clock, ret=%dE\n", ret); > > emits > > ... can't enable I2C clock, ret=EIO > > if ret is -EIO. Petr Mladek (i.e. one of the printk maintainers) had > concerns if this would be well received and worth the effort. He asked > to present it to a few subsystems. So for now, this patch converting the > i2c-imx driver shouldn't be applied yet but it would be great to get > some feedback about if you think that being able to easily printk (for > example) "EIO" instead of "-5" is a good idea. Would it help you? Do you > think it helps your users? Yes, it would help me. And users, too, I am quite sure. For me, if I mix up two numbers while debugging, I am hunting ghosts for a while until I realize my mistake. So: Acked-by: Wolfram Sang I think the main drawback is that ERRORCODES in vsprintf.c now need maintenance, but I think it is worth the effort. I'd be interested in the overhead in size this causes, but I also think it is worth the effort. (It could even be compiled out if we have some generic Kconfig symbol for smaller kernels). Thanks, Wolfram signature.asc Description: PGP signature
Re: [PATCH 3/5] docs: i2c: convert to ReST and add to driver-api bookset
On Fri, Jun 28, 2019 at 06:23:14PM -0300, Mauro Carvalho Chehab wrote: > Convert each file at I2C subsystem, renaming them to .rst and > adding to the driver-api book. > > Signed-off-by: Mauro Carvalho Chehab I glimpsed over it and it looks basically OK. I won't have time to actually review all of this. But I trust you and we can fix things later. So: Acked-by: Wolfram Sang I assume this goes in via your or doc-tree? > Next/merge.log| 6 +- This file doesn't exist upstream, though. signature.asc Description: PGP signature
Re: [PATCH 10/10] docs: fix broken documentation links
On Mon, May 20, 2019 at 11:47:39AM -0300, Mauro Carvalho Chehab wrote: > Mostly due to x86 and acpi conversion, several documentation > links are still pointing to the old file. Fix them. > > Signed-off-by: Mauro Carvalho Chehab Thanks, didn't notice that. > Documentation/i2c/instantiating-devices | 2 +- ... > diff --git a/Documentation/i2c/instantiating-devices > b/Documentation/i2c/instantiating-devices > index 0d85ac1935b7..5a3e2f331e8c 100644 > --- a/Documentation/i2c/instantiating-devices > +++ b/Documentation/i2c/instantiating-devices > @@ -85,7 +85,7 @@ Method 1c: Declare the I2C devices via ACPI > --- > > ACPI can also describe I2C devices. There is special documentation for this > -which is currently located at Documentation/acpi/enumeration.txt. > +which is currently located at > Documentation/firmware-guide/acpi/enumeration.rst. > > > Method 2: Instantiate the devices explicitly For this I2C part: Reviewed-by: Wolfram Sang signature.asc Description: PGP signature
[PATCH] Documentation: gpio: driver: fix wire name for I2C
Typo: the data line is called "SDA" not "SCA". Signed-off-by: Wolfram Sang --- Documentation/driver-api/gpio/driver.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/driver-api/gpio/driver.rst b/Documentation/driver-api/gpio/driver.rst index a92d8837b62b..3043167fc557 100644 --- a/Documentation/driver-api/gpio/driver.rst +++ b/Documentation/driver-api/gpio/driver.rst @@ -135,7 +135,7 @@ This configuration is normally used as a way to achieve one of two things: - inverse wire-OR on an I/O line, for example a GPIO line, making it possible for any driving stage on the line to drive it low even if any other output to the same line is simultaneously driving it high. A special case of this - is driving the SCL and SCA lines of an I2C bus, which is by definition a + is driving the SCL and SDA lines of an I2C bus, which is by definition a wire-OR bus. Both usecases require that the line be equipped with a pull-up resistor. This -- 2.11.0
Re: [PATCH v10 0/9] Add the I3C subsystem
Hi Boris, > What we could do though, is expose I3C devices that do not have a > driver in kernel space, like spidev does. ... > Mark, Wolfram, Arnd, Greg, any opinion? Is there a benefit for having drivers in userspace? My gut feeling is to encourage people to write kernel drivers. If this is, for some reason, not possible for some driver, then we have a use case at hand to test the then-to-be-developed userspace interface against. Until then, I personally wouldn't waste effort on designing it without a user in sight. Dunno if you have that, but a debug interface (exchanging data with clients) on the other hand would be super useful most probably. Maybe you can start having that in debugfs and already learn from it if you ever want to move some interface outside of debugfs? Kind regards, Wolfram signature.asc Description: PGP signature
Re: [PATCH 0/3] Add driver for Synopsys DesignWare I3C master IP
Boris, > This patch series is a proposal for the I3C master driver for Synopsys IP. Any news on the I3C mailing list? It is not much yet, still I was wondering... signature.asc Description: PGP signature
[PATCH 1/2] MAINTAINERS: sort excludes for Documentation
Helps reading and hopefully avoids duplicates. Also, consistently add the trailing '/' to make clear those are directories. Signed-off-by: Wolfram Sang --- MAINTAINERS | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/MAINTAINERS b/MAINTAINERS index 9ad052aeac39..2698ee553008 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -4485,11 +4485,11 @@ S: Maintained F: Documentation/ F: scripts/kernel-doc X: Documentation/ABI/ +X: Documentation/acpi/ X: Documentation/devicetree/ -X: Documentation/acpi -X: Documentation/power -X: Documentation/spi -X: Documentation/media +X: Documentation/media/ +X: Documentation/power/ +X: Documentation/spi/ T: git git://git.lwn.net/linux.git docs-next DOCUMENTATION/ITALIAN -- 2.18.0
[PATCH 2/2] MAINTAINERS: add i2c to the excludes for Documentation
I'll handle these myself but thanks for providing the fallback! Signed-off-by: Wolfram Sang --- MAINTAINERS | 1 + 1 file changed, 1 insertion(+) diff --git a/MAINTAINERS b/MAINTAINERS index 2698ee553008..39c39dd6fba6 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -4487,6 +4487,7 @@ F:scripts/kernel-doc X: Documentation/ABI/ X: Documentation/acpi/ X: Documentation/devicetree/ +X: Documentation/i2c/ X: Documentation/media/ X: Documentation/power/ X: Documentation/spi/ -- 2.18.0
Re: [PATCH v5 3/5] i2c: i2c-qcom-geni: Add bus driver for the Qualcomm GENI I2C controller
Hi,
On Fri, Mar 23, 2018 at 02:20:59PM -0600, Karthikeyan Ramasubramanian wrote:
> This bus driver supports the GENI based i2c hardware controller in the
> Qualcomm SOCs. The Qualcomm Generic Interface (GENI) is a programmable
> module supporting a wide range of serial interfaces including I2C. The
> driver supports FIFO mode and DMA mode of transfer and switches modes
> dynamically depending on the size of the transfer.
>
> Signed-off-by: Karthikeyan Ramasubramanian
> Signed-off-by: Sagar Dharia
> Signed-off-by: Girish Mahadevan
Is one of these people interested in maintaining this driver? Then, an
entry for MAINTAINERS would be needed, too. (Same goes for
drivers/soc/qcom/ IMHO, but this is not my realm, so just saying)
> +static const struct geni_i2c_err_log gi2c_log[] = {
> + [GP_IRQ0] = {-EINVAL, "Unknown I2C err GP_IRQ0"},
> + [NACK] = {-ENOTCONN, "NACK: slv unresponsive, check its
> power/reset-ln"},
> + [GP_IRQ2] = {-EINVAL, "Unknown I2C err GP IRQ2"},
> + [BUS_PROTO] = {-EPROTO, "Bus proto err, noisy/unepxected start/stop"},
> + [ARB_LOST] = {-EBUSY, "Bus arbitration lost, clock line undriveable"},
> + [GP_IRQ5] = {-EINVAL, "Unknown I2C err GP IRQ5"},
> + [GENI_OVERRUN] = {-EIO, "Cmd overrun, check GENI cmd-state machine"},
> + [GENI_ILLEGAL_CMD] = {-EILSEQ, "Illegal cmd, check GENI cmd-state
> machine"},
> + [GENI_ABORT_DONE] = {-ETIMEDOUT, "Abort after timeout successful"},
> + [GENI_TIMEOUT] = {-ETIMEDOUT, "I2C TXN timed out"},
> +};
Please check Documentation/i2c/fault-codes for better -ERRNO values,
especially for NACK and ARB_LOST.
Rest looks good from a glimpse.
Thanks,
Wolfram
signature.asc
Description: PGP signature
Re: [PATCH 0/7] i2c: clean up include/linux/i2c-*
On Thu, Apr 19, 2018 at 10:00:06PM +0200, Wolfram Sang wrote: > Move all plain platform_data includes to the platform_data-dir > (except for i2c-pnx which can be moved into the driver itself). > > My preference is to take these patches via the i2c tree. I can provide an > immutable branch if needed. But we can also discuss those going in via > arch-trees if dependencies are against us. All applied to for-next! The immutable branch is here: git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git i2c/platform_data-immutable Thanks, Wolfram signature.asc Description: PGP signature
[PATCH 2/7] i2c: i2c-mux-gpio: move header to platform_data
This header only contains platform_data. Move it to the proper directory.
Signed-off-by: Wolfram Sang
---
Documentation/i2c/muxes/i2c-mux-gpio | 4 ++--
MAINTAINERS | 2 +-
drivers/i2c/busses/i2c-i801.c| 2 +-
drivers/i2c/muxes/i2c-mux-gpio.c | 2 +-
include/linux/{ => platform_data}/i2c-mux-gpio.h | 0
5 files changed, 5 insertions(+), 5 deletions(-)
rename include/linux/{ => platform_data}/i2c-mux-gpio.h (100%)
diff --git a/Documentation/i2c/muxes/i2c-mux-gpio
b/Documentation/i2c/muxes/i2c-mux-gpio
index 7a8d7d261632..893ecdfe6e43 100644
--- a/Documentation/i2c/muxes/i2c-mux-gpio
+++ b/Documentation/i2c/muxes/i2c-mux-gpio
@@ -30,12 +30,12 @@ i2c-mux-gpio uses the platform bus, so you need to provide
a struct
platform_device with the platform_data pointing to a struct
i2c_mux_gpio_platform_data with the I2C adapter number of the master
bus, the number of bus segments to create and the GPIO pins used
-to control it. See include/linux/i2c-mux-gpio.h for details.
+to control it. See include/linux/platform_data/i2c-mux-gpio.h for details.
E.G. something like this for a MUX providing 4 bus segments
controlled through 3 GPIO pins:
-#include
+#include
#include
static const unsigned myboard_gpiomux_gpios[] = {
diff --git a/MAINTAINERS b/MAINTAINERS
index 7aad64b62102..44b4bb5cf94e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5879,7 +5879,7 @@ M:Peter Korsgaard
L: linux-...@vger.kernel.org
S: Supported
F: drivers/i2c/muxes/i2c-mux-gpio.c
-F: include/linux/i2c-mux-gpio.h
+F: include/linux/platform_data/i2c-mux-gpio.h
F: Documentation/i2c/muxes/i2c-mux-gpio
GENERIC HDLC (WAN) DRIVERS
diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
index e0d59e9ff3c6..bff160d1ce3f 100644
--- a/drivers/i2c/busses/i2c-i801.c
+++ b/drivers/i2c/busses/i2c-i801.c
@@ -106,7 +106,7 @@
#if IS_ENABLED(CONFIG_I2C_MUX_GPIO) && defined CONFIG_DMI
#include
-#include
+#include
#endif
/* I801 SMBus address offsets */
diff --git a/drivers/i2c/muxes/i2c-mux-gpio.c b/drivers/i2c/muxes/i2c-mux-gpio.c
index 1a9973ede443..15a7cc0459fb 100644
--- a/drivers/i2c/muxes/i2c-mux-gpio.c
+++ b/drivers/i2c/muxes/i2c-mux-gpio.c
@@ -10,7 +10,7 @@
#include
#include
-#include
+#include
#include
#include
#include
diff --git a/include/linux/i2c-mux-gpio.h
b/include/linux/platform_data/i2c-mux-gpio.h
similarity index 100%
rename from include/linux/i2c-mux-gpio.h
rename to include/linux/platform_data/i2c-mux-gpio.h
--
2.11.0
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/7] i2c: i2c-ocores: move header to platform_data
This header only contains platform_data. Move it to the proper directory.
Signed-off-by: Wolfram Sang
---
Documentation/i2c/busses/i2c-ocores| 2 +-
drivers/i2c/busses/i2c-ocores.c| 2 +-
drivers/mfd/timberdale.c | 2 +-
include/linux/{ => platform_data}/i2c-ocores.h | 0
4 files changed, 3 insertions(+), 3 deletions(-)
rename include/linux/{ => platform_data}/i2c-ocores.h (100%)
diff --git a/Documentation/i2c/busses/i2c-ocores
b/Documentation/i2c/busses/i2c-ocores
index c269aaa2f26a..c12fa9d3b050 100644
--- a/Documentation/i2c/busses/i2c-ocores
+++ b/Documentation/i2c/busses/i2c-ocores
@@ -18,7 +18,7 @@ Usage
i2c-ocores uses the platform bus, so you need to provide a struct
platform_device with the base address and interrupt number. The
dev.platform_data of the device should also point to a struct
-ocores_i2c_platform_data (see linux/i2c-ocores.h) describing the
+ocores_i2c_platform_data (see linux/platform_data/i2c-ocores.h) describing the
distance between registers and the input clock speed.
There is also a possibility to attach a list of i2c_board_info which
the i2c-ocores driver will add to the bus upon creation.
diff --git a/drivers/i2c/busses/i2c-ocores.c b/drivers/i2c/busses/i2c-ocores.c
index 8c42ca7107b2..d7da9adf7ee1 100644
--- a/drivers/i2c/busses/i2c-ocores.c
+++ b/drivers/i2c/busses/i2c-ocores.c
@@ -21,7 +21,7 @@
#include
#include
#include
-#include
+#include
#include
#include
#include
diff --git a/drivers/mfd/timberdale.c b/drivers/mfd/timberdale.c
index cd4a6d7d6750..118d7ef727e6 100644
--- a/drivers/mfd/timberdale.c
+++ b/drivers/mfd/timberdale.c
@@ -30,7 +30,7 @@
#include
#include
-#include
+#include
#include
#include
diff --git a/include/linux/i2c-ocores.h
b/include/linux/platform_data/i2c-ocores.h
similarity index 100%
rename from include/linux/i2c-ocores.h
rename to include/linux/platform_data/i2c-ocores.h
--
2.11.0
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/7] i2c: clean up include/linux/i2c-*
Move all plain platform_data includes to the platform_data-dir
(except for i2c-pnx which can be moved into the driver itself).
My preference is to take these patches via the i2c tree. I can provide an
immutable branch if needed. But we can also discuss those going in via
arch-trees if dependencies are against us.
The current branch can be found here:
git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git i2c/platform_data
and buildbot had no complaints.
Looking forward to comments or Acks, Revs...
Kind regards,
Wolfram
Wolfram Sang (7):
i2c: i2c-gpio: move header to platform_data
i2c: i2c-mux-gpio: move header to platform_data
i2c: i2c-ocores: move header to platform_data
i2c: i2c-omap: move header to platform_data
i2c: i2c-pca-platform: move header to platform_data
i2c: i2c-xiic: move header to platform_data
i2c: pnx: move header into the driver
Documentation/i2c/busses/i2c-ocores| 2 +-
Documentation/i2c/muxes/i2c-mux-gpio | 4 +--
MAINTAINERS| 8 ++---
arch/arm/mach-ks8695/board-acs5k.c | 2 +-
arch/arm/mach-omap1/board-htcherald.c | 2 +-
arch/arm/mach-omap1/common.h | 2 +-
arch/arm/mach-omap1/i2c.c | 2 +-
arch/arm/mach-omap2/common.h | 2 +-
arch/arm/mach-omap2/omap_hwmod_2420_data.c | 2 +-
arch/arm/mach-omap2/omap_hwmod_2430_data.c | 2 +-
arch/arm/mach-omap2/omap_hwmod_33xx_data.c | 2 +-
arch/arm/mach-omap2/omap_hwmod_3xxx_data.c | 2 +-
arch/arm/mach-omap2/omap_hwmod_44xx_data.c | 2 +-
arch/arm/mach-omap2/omap_hwmod_54xx_data.c | 2 +-
arch/arm/mach-omap2/omap_hwmod_7xx_data.c | 2 +-
arch/arm/mach-pxa/palmz72.c| 2 +-
arch/arm/mach-pxa/viper.c | 2 +-
arch/arm/mach-sa1100/simpad.c | 2 +-
arch/mips/alchemy/board-gpr.c | 2 +-
arch/sh/boards/board-sh7785lcr.c | 2 +-
drivers/i2c/busses/i2c-gpio.c | 2 +-
drivers/i2c/busses/i2c-i801.c | 2 +-
drivers/i2c/busses/i2c-ocores.c| 2 +-
drivers/i2c/busses/i2c-omap.c | 2 +-
drivers/i2c/busses/i2c-pca-platform.c | 2 +-
drivers/i2c/busses/i2c-pnx.c | 21 +++-
drivers/i2c/busses/i2c-xiic.c | 2 +-
drivers/i2c/muxes/i2c-mux-gpio.c | 2 +-
drivers/media/platform/marvell-ccic/mmp-driver.c | 2 +-
drivers/mfd/sm501.c| 2 +-
drivers/mfd/timberdale.c | 4 +--
include/linux/i2c-pnx.h| 38 --
include/linux/{ => platform_data}/i2c-gpio.h | 0
include/linux/{ => platform_data}/i2c-mux-gpio.h | 0
include/linux/{ => platform_data}/i2c-ocores.h | 0
include/linux/{ => platform_data}/i2c-omap.h | 0
.../linux/{ => platform_data}/i2c-pca-platform.h | 0
include/linux/{ => platform_data}/i2c-xiic.h | 0
38 files changed, 55 insertions(+), 74 deletions(-)
delete mode 100644 include/linux/i2c-pnx.h
rename include/linux/{ => platform_data}/i2c-gpio.h (100%)
rename include/linux/{ => platform_data}/i2c-mux-gpio.h (100%)
rename include/linux/{ => platform_data}/i2c-ocores.h (100%)
rename include/linux/{ => platform_data}/i2c-omap.h (100%)
rename include/linux/{ => platform_data}/i2c-pca-platform.h (100%)
rename include/linux/{ => platform_data}/i2c-xiic.h (100%)
--
2.11.0
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 2/3] Documentation/i2c: sync docs with current state of i2c-tools
> +The above functions are made available by linking against the libi2c library, > +which is provided by the i2c-tools project. See: > +https://git.kernel.org/pub/scm/utils/i2c-tools/i2c-tools.git/. In the beginning, we say that '#include ' is needed. Shouldn't we mention i2c-tools there already and in what case it is needed (only for SMBus)? I'd think so. Sam, would you be open to do this as an incremental patch? signature.asc Description: PGP signature
Re: [PATCH v3 3/3] Documentation/i2c: adopt kernel commenting style in examples
On Fri, Apr 13, 2018 at 10:42:57AM -0700, Sam Hansen wrote: > The example I2C code is rewritten to adopt the preferred kernel block > commenting style. > > Signed-off-by: Sam Hansen Applied to for-current, thanks! signature.asc Description: PGP signature
Re: [PATCH v3 2/3] Documentation/i2c: sync docs with current state of i2c-tools
On Fri, Apr 13, 2018 at 10:42:56AM -0700, Sam Hansen wrote: > Currently, Documentation/i2c/dev-interface describes the use of > i2c_smbus_* helper routines as static inlined functions provided by > linux/i2c-dev.h. Work has been done to refactor the linux/i2c-dev.h file > in the i2c-tools project out into its own library. As a result, these > docs have become stale. > > This patch corrects the discrepancy and directs the reader to the > i2c-tools project for more information. > > Signed-off-by: Sam Hansen Applied to for-current, thanks! signature.asc Description: PGP signature
Re: [PATCH v3 1/3] Documentation/i2c: whitespace cleanup
On Fri, Apr 13, 2018 at 10:42:55AM -0700, Sam Hansen wrote: > This strips trailing whitespace in Documentation/i2c/dev-interface. > > Signed-off-by: Sam Hansen Applied to for-current, thanks! signature.asc Description: PGP signature
Re: [PATCH] Documentation/i2c: sync docs with current state of i2c-tools.
> (also, did I send the v3 patch series threaded correctly?) Yes, that worked. Thanks! Things to improve there: I was both in To: and CC: field, so I got mails twice. Jean was missing completely. signature.asc Description: PGP signature
Re: [PATCH v2 3/3] Documentation/i2c: adopt kernel commenting style in examples
> - int adapter_nr = 2; /* probably dynamically determined */ Such comments are actually OK. > -/* ERROR HANDLING; you can check errno to see what went wrong */ Such as well. > - /* Using I2C Write, equivalent of > - i2c_smbus_write_word_data(file, reg, 0x6543) */ > + /* > + * Using I2C Write, equivalent of > + * i2c_smbus_write_word_data(file, reg, 0x6543). > + */ This is the only broken one AFAICT. signature.asc Description: PGP signature
Re: [PATCH v2 2/3] Documentation/i2c: sync docs with current state of i2c-tools
On Fri, Apr 13, 2018 at 09:44:04AM -0700, Sam Hansen wrote: > Currently, Documentation/i2c/dev-interface describes the use of > i2c_smbus_* helper routines as static inlined functions provided by > linux/i2c-dev.h. Work has been done to refactor the linux/i2c-dev.h file > in the i2c-tools project out into its own library. As a result, these > docs have become stale. > > This patch corrects the discrepancy and directs the reader to the > i2c-tools project for more information. > > Signed-off-by: Sam Hansen Looks good to me as well. signature.asc Description: PGP signature
Re: [PATCH v2 1/3] Documentation/i2c: whitespace cleanup
On Fri, Apr 13, 2018 at 09:44:03AM -0700, Sam Hansen wrote: > This strips trailing whitespace in Documentation/i2c/dev-interface. > > Signed-off-by: Sam Hansen Looks good to me. But please send new series as seperate threads. signature.asc Description: PGP signature
Re: [PATCH] Documentation/i2c: sync docs with current state of i2c-tools.
Hi, On Thu, Apr 12, 2018 at 02:33:42PM -0700, Sam Hansen wrote: > Currently, Documentation/i2c/dev-interface describes the use of i2c_smbus_* > helper routines as static inlined functions provided by linux/i2c-dev.h. Work > has been done to refactor the linux/i2c-dev.h file in the i2c-tools project > out into its own library. As a result, these docs have become stale. Thanks for fixing this! > This patch corrects the discrepancy and directs the reader to the i2c-tools > project for more information. Additionally, some trailing-whitespace cleanups > were made. Minor nit: Having the whitespace changes in a seperate patch is a tad easier to review. > - /* Using I2C Write, equivalent of > + /* Using I2C Write, equivalent of > i2c_smbus_write_word_data(file, reg, 0x6543) */ Maybe change to Kernel coding style comments while here? > - Not meant to be called directly; instead, use the access functions > - below. > + If possible, use the provided i2c_smbus_* methods described below in favor > + of issuing direct ioctls. Why this change? > -The above functions are all inline functions, that resolve to calls to > -the i2c_smbus_access function, that on its turn calls a specific ioctl > -with the data in a specific format. Read the source code if you > -want to know what happens behind the screens. > +The above functions are made available by linking against the libi2c library, > +which is provided by the i2c-tools project. See: > +https://git.kernel.org/pub/scm/utils/i2c-tools/i2c-tools.git/. This is fine with me. Maybe Jean has a comment on this? Kind regards, Wolfram signature.asc Description: PGP signature
Re: [PATCH v3 01/11] i2c: Export of_i2c_get_board_info()
> > - info.archdata = &dev_ad; > > Why did you drop this? If the removal is safe, it should be a seperate patch, I mean. signature.asc Description: PGP signature
Re: [PATCH v3 01/11] i2c: Export of_i2c_get_board_info()
Hi Boris, > - rebase on v4.15-rc1 This code has changed a little meanwhile. Please check my for-next branch. Some changes are identical, some similar. > - info.archdata = &dev_ad; Why did you drop this? Regards, Wolfram signature.asc Description: PGP signature
Re: [PATCH] Documentation: i2c: drop unnecessary .owner field in examples
On Mon, Jan 15, 2018 at 10:24:52PM +0200, Andy Shevchenko wrote: > On Mon, Jan 15, 2018 at 2:08 PM, Nicholas Mc Guire wrote: > > From: Nicholas Mc Guire > > > > Currently there are a few drivers that still set the .owner > > in the i2c_driver structure - all of which are reported by > > coccinelle (scripts/coccinelle/api/platform_no_drv_owner.cocci) > > and there are no cases that set the .onwer and do not call any > > of the functions that set the .owner field anyway in any of the > > drivers (checked by a modified coccinelle script based on the > > above) so it seems that the examples are no longer valid and > > .owner = THIS_MODULE, can be removed here. > > > > While at it an obvious typo (new new) was also fixed. > > AFAIU It is right only in case when someone does this, e.g. > module_i2c_driver() macro. Otherwise the field is pretty valid and > must be filled. It gets filled with i2c_add_driver. module_i2c_driver uses i2c_add_driver. I was about to suggest to keep the field in the old driver and describe that it can be removed when using one of i2c_add_driver or module_i2c_driver. But then I realised that the kernel tree does not have any such old drivers anymore and I couldn't even find out-of-tree code via some search engines (I tried looking for "I2C_CLIENT_INSMOD"). I consider this obsolete and irrelevant these days. It might be good to simply remove it to not confuse users. Thoughts? signature.asc Description: PGP signature
Re: [RFC 0/5] Add I3C subsystem
> MIPI has opened the I3C spec [1], it can be downloaded here [2]. Wow, that's good news. And so fast. Congrats and thanks a lot! signature.asc Description: PGP signature
Re: [PATCH 11/12] PM: i2c-designware-platdrv: Optimize power management
On Mon, Oct 16, 2017 at 03:31:17AM +0200, Rafael J. Wysocki wrote: > From: Rafael J. Wysocki > > Optimize the power management in i2c-designware-platdrv by making it > set the DPM_FLAG_SMART_SUSPEND and DPM_FLAG_LEAVE_SUSPENDED which > allows some code to be dropped from its PM callbacks. > > First, setting DPM_FLAG_SMART_SUSPEND causes the intel-lpss driver > to avoid resuming i2c-designware-platdrv devices in its ->prepare > callback, so they can stay in runtime suspend after that point even > if the direct-complete feature is not used for them. > > It also causes the PM core to avoid invoking "late" and "noirq" > suspend callbacks for these devices if they are in runtime suspend > at the beginning of the "late" phase of device suspend during > system suspend. That guarantees dw_i2c_plat_suspend() to be > called for a device only if it is not in runtime suspend. > Moreover, it also causes the PM core to set the device's runtime > PM status to "active" after calling dw_i2c_plat_resume() for > it, so the driver doesn't need internal flags to avoid invoking > either dw_i2c_plat_suspend() or dw_i2c_plat_resume() twice in > a row. > > Second, setting DPM_FLAG_LEAVE_SUSPENDED enables the optimization > allowing the device to stay suspended after system resume under > suitable conditions, so again the driver doesn't need to take > care of that by itself. > > Accordingly, the internal "suspended" and "skip_resume" flags > used by the driver are not necessary any more, so drop them and > simplify the driver's PM callbacks. > > Additionally, notice that dw_i2c_plat_complete() only needs > to schedule runtime PM for the device if platform firmware > has been involved in resuming the system, so make it call > pm_resume_via_firmware() to check that. > > Signed-off-by: Rafael J. Wysocki So, if the designware maintainers ack it, I will, too. signature.asc Description: PGP signature
Re: Documentation: add Kernel Driver Statement to the kernel
> Not that sphinx doesn't have it's own issues, but you have to admit it > is much better now than it used to be, right? That goes without saying, but we still added plain textfiles to Documentation/ since 2008, so I was wondering... signature.asc Description: PGP signature
Re: Documentation: add Kernel Driver Statement to the kernel
On Fri, Oct 06, 2017 at 11:10:38AM +0200, gre...@linuxfoundation.org wrote: > Way back in 2008 we didn't have "robust" in-kernel documentation system, > so the idea of putting something like the kernel driver statement in the > kernel tree wasn't even imagined. But now that has changed, so add the > old document to the kernel source itself to allow for us to properly > reference it in one canonical place (as the LF wiki keeps moving things > around.) Cool, I like it much to see it added to the kernel tree. But could you explain what "robust" means in this context? And what has changed which makes it "robust"? Sphinx? I am interested in how such documents are handled best. signature.asc Description: PGP signature
Re: [RFC 2/5] i3c: Add core I3C infrastructure
> > Yes, my wording was a bit too strong. It is possible, sure. Yet, I > > understood that one of the features of I3C is to have in-band interrupt > > support. We will see if the demand for backward compatibility or "saving > > pins" is higher. > > > > Indeed, you can use in-band interrupts if your device is able to > generate them, but that doesn't prevent I3C device designers from using > an external pin to signal interrupts if they prefer. Exactly. Thus, "We will see if the demand for backward compatibility or "saving pins" is higher" :) signature.asc Description: PGP signature
Re: [RFC 2/5] i3c: Add core I3C infrastructure
> I'm surprised they didn't allow for slave clock stretching when > communicating with a legacy i2c device, it will prohibit use of a rather > large class of devices. :( Yes, but I3C is push/pull IIRC. > As for interrupts you are always free to wire up an out-of-band > interrupt like before. :) Yes, my wording was a bit too strong. It is possible, sure. Yet, I understood that one of the features of I3C is to have in-band interrupt support. We will see if the demand for backward compatibility or "saving pins" is higher. signature.asc Description: PGP signature
Re: [RFC 2/5] i3c: Add core I3C infrastructure
> I do not know of any real devices as of today (all my tests have been > done with a dummy/fake I3C slaves emulated with a slave IP), I see. > spec clearly describe what legacy/static addresses are for and one of > their use case is to connect an I3C device on an I2C bus and let it act > as an I2C device. OK. That makes it more likely. > Unless you want your device (likely a sensor) to be compatible with both > I3C and I2C so that you can target even more people. Right. My question was if this is a realistic or more academic scenario. > I'm perfectly fine with the I3C / I2C framework separation. The only > minor problem I had with that was the inaccuracy of the > sysfs/device-model representation: we don't have one i2c and one i3c > bus, we just have one i3c bus with a mix of i2c and i3c devices. I understand that. What if I2C had the same seperation between the "bus" and the "master"? signature.asc Description: PGP signature
Re: [RFC 2/5] i3c: Add core I3C infrastructure
> > The second way is to have a number of #ifdef and complex > > Kconfig dependencies for the driver to only register the > > device_driver objects for the buses that are enabled. This > > is also doable, but everyone gets the logic wrong the first time. > > Hm, I understand now why you'd prefer to have a single bus. Can't we > solve this problem with a module_i3c_i2c_driver() macro that would hide > all this complexity from I2C/I3C drivers? Do you know of devices speaking both i3c and i2c as of today? I think I3C/I2C is a bit different than I2C/SPI. For the latter, it might happen that you have only this or that bus on the board, so it makes sense to support both. But if you have I3C, you can simply attach the I2C device onto it. I guess you would only implement I3C in the device if you explicitly need its feature set. And then, a I2C fallback doesn't make much sense? Or am I missing something? OK, now I know that those I3C+I2C devices will exist, even if only for Murphy's law. However, my assumptions would be that those devices are not common and so we could live with the core plus bus_drivers seperation we have for SPI/I2C already (although I would love a common regmap-based I2C/SPI abstraction). signature.asc Description: PGP signature
Re: [RFC 2/5] i3c: Add core I3C infrastructure
> Actually, that's the first option I considered, but I3C and I2C are > really different. I'm not talking about the physical layer here, but > the way the bus has to be handled by the software layer. Actually, I > thing the I3C bus is philosophically closer to auto-discoverable busses > like USB than I2C or SPI. Acked-by: Wolfram Sang > Of course, I can move all the code in drivers/i2c/, but that won't > change the fact that I3C and I2C busses are completely different > with little to share between them. That wouldn't make sense. > To me, the I2C backward compatibility is just a nice feature that was > added to help people smoothly transition from mixed I3C busses with > both I2C and I3C devices connected to it (I2C devices being here > when no (affordable) equivalent exist in the I3C world) to pure I3C > busses with only I3C devices connected to it. Yeah, and it is still to be seen how good this really works. Devices which do clock stretching are out of the question. Probably everything which needs an interrupt as well? > This being said, I'd be happy if you prove me wrong and propose a > solution that allows us to extend the I2C framework to support I3C > without to much pain ;-). From all I know, I don't see that coming. signature.asc Description: PGP signature
Re: [RFC 0/5] Add I3C subsystem
> > I agree this is the least invasive and also the most compatible > > approach. The other solution would probably be to have some kind of > > emulation layer? > > Could you detail a bit more what you mean by "emulation layer"? Not really. That was more a extremly high level approach of what theoretically could be possible. When I try to think about details, it gets pretty invasive. > > Since the spec is not public, details about the protocol will be > > especially useful, I'd say. > > Okay, I'll see what I can do. Thanks. signature.asc Description: PGP signature
Re: [RFC 0/5] Add I3C subsystem
Hi Boris, > This patch series is a proposal for a new I3C [1] subsystem. Nice. Good luck with that! Some hi-level comments from me related to I2C. I can't say a lot more because the specs are not public :( > - the bus element is a separate object and is not implicitly described > by the master (as done in I2C). The reason is that I want to be able > to handle multiple master connected to the same bus and visible to > Linux. > In this situation, we should only have one instance of the device and > not one per master, and sharing the bus object would be part of the > solution to gracefully handle this case. > I'm not sure if we will ever need to deal with multiple masters > controlling the same bus and exposed under Linux, but separating the > bus and master concept is pretty easy, hence the decision to do it > now, just in case we need it some day. From my experience, it is a good thing to have this separation. > - I2C backward compatibility has been designed to be transparent to I2C > drivers and the I2C subsystem. The I3C master just registers an I2C > adapter which creates a new I2C bus. I'd say that, from a > representation PoV it's not ideal because what should appear as a > single I3C bus exposing I3C and I2C devices here appears as 2 > different busses connected to each other through the parenting (the > I3C master is the parent of the I2C and I3C busses). > On the other hand, I don't see a better solution if we want something > that is not invasive. I agree this is the least invasive and also the most compatible approach. The other solution would probably be to have some kind of emulation layer? > I'd also like to get feedback on the doc. Should I detail a bit more > the protocol or the framework API? Is this the kind of things you > expect in a subsystem doc? Since the spec is not public, details about the protocol will be especially useful, I'd say. Regards, Wolfram signature.asc Description: PGP signature
Re: [RFC 2/5] i3c: Add core I3C infrastructure
> +This document is just a brief introduction to the I3C protocol and the > concepts > +it brings on the table. If you need more information, please refer to the > MIPI > +I3C specification. I wish I could. > + > +Introduction > + > + > +The I3C (I-Cube-C) is a MIPI standardized protocol designed to overcome I2C "Eye-three-See", according to: http://eecatalog.com/sensors/2017/07/05/after-35-years-of-i2c-i3c-improves-capability-and-performance/ > +Backward compatibility with I2C devices > +=== > + > +The I3C protocol has been designed to be backward compatible with I2C > devices. > +This backward compatibility allows one to connect a mix of I2C and I3C > devices > +on the same bus, though, in order to be really efficient, I2C devices should > +be equipped with 50 ns spike filters. I just found a slide which says I3C does not support clock stretching. That should be mentioned here, too. signature.asc Description: PGP signature
Re: [PATCH v15 00/13] mux controller abstraction and iio/i2c muxes
> > All now queued up, nice work, thanks for sticking with this. > > *big sigh of relief* I can imagine. Great work, Peda! Congrats. signature.asc Description: PGP signature
Re: [PATCH 0/8] i2c: refactor core and break out blocks
On Fri, May 26, 2017 at 10:20:51AM +0200, Wolfram Sang wrote: > Yes, I wanted to do this for years now... The I2C core became a huge > monolithic > blob getting harder and harder to maintain. This series breaks out some > functional parts into seperate files. This makes the code easier to handle > because of the smaller chunks. It reduces ifdeffery because we can now handle > compilation at the Makefile level. And it helps to spread responsibility, e.g. > the ACPI maintainers do now have a dedicated file listed in MAINTAINERS. > > This series was tested with a Renesas Lager board (R-Car H2 SoC). It booted > normally and all device drivers for I2C clients seem to work normally. I wired > two I2C busses together and used i2c-slave-eeprom to let one I2C IP core read > out data from the other. That all worked fine. Buildbot is also happy, it > found > two issues of the first (non public) iteration. Thanks! > > I did not test ACPI and hope for some assistance here :) I'd also be happy if > people could check the includes of the newly created files, there might be > missing some. > > As a result, the main i2c-core file goes down from ~3600 lines to ~2000 lines. > I think this is pretty helpful. I plan to apply this for v4.13 to not block > other core changes. Let's see if we are there yet and the series is ready. > Looking forward to comments. > > A branch can be found here: > git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git i2c/core-refactor > So, I pushed this series out into -next to get broader testing. Everything went well, so far. Might be famous last words... ;) signature.asc Description: PGP signature
[PATCH] Documentation: DMA API: fix a typo in a function name
Correct the typo, the wrongly typed function does not exist.
Fixes: 6c9c6d6301287e ("dma-debug: New interfaces to debug dma mapping errors")
Signed-off-by: Wolfram Sang
---
Documentation/DMA-API.txt | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/DMA-API.txt b/Documentation/DMA-API.txt
index 6b20128fab8a2b..71200dfa09228b 100644
--- a/Documentation/DMA-API.txt
+++ b/Documentation/DMA-API.txt
@@ -692,7 +692,7 @@ of preallocated entries is defined per architecture. If it
is too low for you
boot with 'dma_debug_entries=' to overwrite the
architectural default.
-void debug_dmap_mapping_error(struct device *dev, dma_addr_t dma_addr);
+void debug_dma_mapping_error(struct device *dev, dma_addr_t dma_addr);
dma-debug interface debug_dma_mapping_error() to debug drivers that fail
to check DMA mapping errors on addresses returned by dma_map_single() and
--
2.11.0
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/8] i2c: refactor core and break out blocks
> If you don't mind sending the whole series to the intel-gfx list (Cc'd), > our CI will run a bunch of tests on it, exercising our use of the I2C > adapter interfaces for display data channel and I2C over Display Port > native aux. Cool, that sounds very helpful! Thanks for the offer, I'll resend the series there as RFT. signature.asc Description: PGP signature
[PATCH 1/8] i2c: rename core source file to allow refactorization
The I2C core became quite huge and its monolithic structure makes
maintenance hard. So, prepare to break out some functionality into
seperate files by renaming the source file. Note that we keep the
resulting object name constant to avoid regressions.
Signed-off-by: Wolfram Sang
---
Documentation/driver-api/i2c.rst| 2 +-
drivers/i2c/Makefile| 4 +++-
drivers/i2c/busses/i2c-designware-core.c| 2 +-
drivers/i2c/{i2c-core.c => i2c-core-base.c} | 0
4 files changed, 5 insertions(+), 3 deletions(-)
rename drivers/i2c/{i2c-core.c => i2c-core-base.c} (100%)
diff --git a/Documentation/driver-api/i2c.rst b/Documentation/driver-api/i2c.rst
index 0bf86a445d0135..67366d9ff7303f 100644
--- a/Documentation/driver-api/i2c.rst
+++ b/Documentation/driver-api/i2c.rst
@@ -41,5 +41,5 @@ i2c_adapter devices which don't support those I2C operations.
.. kernel-doc:: drivers/i2c/i2c-boardinfo.c
:functions: i2c_register_board_info
-.. kernel-doc:: drivers/i2c/i2c-core.c
+.. kernel-doc:: drivers/i2c/i2c-core-base.c
:export:
diff --git a/drivers/i2c/Makefile b/drivers/i2c/Makefile
index 45095b3d16a914..d459c7e5907607 100644
--- a/drivers/i2c/Makefile
+++ b/drivers/i2c/Makefile
@@ -4,6 +4,8 @@
obj-$(CONFIG_I2C_BOARDINFO)+= i2c-boardinfo.o
obj-$(CONFIG_I2C) += i2c-core.o
+i2c-core-objs := i2c-core-base.o
+
obj-$(CONFIG_I2C_SMBUS)+= i2c-smbus.o
obj-$(CONFIG_I2C_CHARDEV) += i2c-dev.o
obj-$(CONFIG_I2C_MUX) += i2c-mux.o
@@ -12,4 +14,4 @@ obj-$(CONFIG_I2C_STUB)+= i2c-stub.o
obj-$(CONFIG_I2C_SLAVE_EEPROM) += i2c-slave-eeprom.o
ccflags-$(CONFIG_I2C_DEBUG_CORE) := -DDEBUG
-CFLAGS_i2c-core.o := -Wno-deprecated-declarations
+CFLAGS_i2c-core-base.o := -Wno-deprecated-declarations
diff --git a/drivers/i2c/busses/i2c-designware-core.c
b/drivers/i2c/busses/i2c-designware-core.c
index c453717b753b72..3c41995634c2f9 100644
--- a/drivers/i2c/busses/i2c-designware-core.c
+++ b/drivers/i2c/busses/i2c-designware-core.c
@@ -583,7 +583,7 @@ i2c_dw_xfer_msg(struct dw_i2c_dev *dev)
*/
/*
-* i2c-core.c always sets the buffer length of
+* i2c-core always sets the buffer length of
* I2C_FUNC_SMBUS_BLOCK_DATA to 1. The length will
* be adjusted when receiving the first byte.
* Thus we can't stop the transaction here.
diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core-base.c
similarity index 100%
rename from drivers/i2c/i2c-core.c
rename to drivers/i2c/i2c-core-base.c
--
2.11.0
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/8] i2c: refactor core and break out blocks
Yes, I wanted to do this for years now... The I2C core became a huge monolithic
blob getting harder and harder to maintain. This series breaks out some
functional parts into seperate files. This makes the code easier to handle
because of the smaller chunks. It reduces ifdeffery because we can now handle
compilation at the Makefile level. And it helps to spread responsibility, e.g.
the ACPI maintainers do now have a dedicated file listed in MAINTAINERS.
This series was tested with a Renesas Lager board (R-Car H2 SoC). It booted
normally and all device drivers for I2C clients seem to work normally. I wired
two I2C busses together and used i2c-slave-eeprom to let one I2C IP core read
out data from the other. That all worked fine. Buildbot is also happy, it found
two issues of the first (non public) iteration. Thanks!
I did not test ACPI and hope for some assistance here :) I'd also be happy if
people could check the includes of the newly created files, there might be
missing some.
As a result, the main i2c-core file goes down from ~3600 lines to ~2000 lines.
I think this is pretty helpful. I plan to apply this for v4.13 to not block
other core changes. Let's see if we are there yet and the series is ready.
Looking forward to comments.
A branch can be found here:
git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git i2c/core-refactor
Kind regards,
Wolfram
Wolfram Sang (8):
i2c: rename core source file to allow refactorization
i2c: break out slave support into seperate file
i2c: break out smbus support into seperate file
i2c: break out OF support into seperate file
i2c: break out ACPI support into seperate file
docs: i2c: dev-interface: adapt to new filenames of the i2c core
i2c: remove unneeded includes from core
i2c: reformat core-base file header
Documentation/driver-api/i2c.rst|5 +-
Documentation/i2c/dev-interface | 14 +-
MAINTAINERS |1 +
drivers/i2c/Makefile|7 +-
drivers/i2c/busses/i2c-designware-core.c|2 +-
drivers/i2c/i2c-core-acpi.c | 653 +++
drivers/i2c/{i2c-core.c => i2c-core-base.c} | 1681 +--
drivers/i2c/i2c-core-of.c | 276 +
drivers/i2c/i2c-core-slave.c| 115 ++
drivers/i2c/i2c-core-smbus.c| 594 ++
drivers/i2c/i2c-core.h | 24 +
include/trace/events/i2c.h | 226 +---
include/trace/events/smbus.h| 249
13 files changed, 1978 insertions(+), 1869 deletions(-)
create mode 100644 drivers/i2c/i2c-core-acpi.c
rename drivers/i2c/{i2c-core.c => i2c-core-base.c} (58%)
create mode 100644 drivers/i2c/i2c-core-of.c
create mode 100644 drivers/i2c/i2c-core-slave.c
create mode 100644 drivers/i2c/i2c-core-smbus.c
create mode 100644 include/trace/events/smbus.h
--
2.11.0
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/8] i2c: break out smbus support into seperate file
Break out the exported SMBus functions and the emulation layer into a
seperate file. This also involved splitting up the tracing header into
an I2C and an SMBus part.
Signed-off-by: Wolfram Sang
---
Documentation/driver-api/i2c.rst | 3 +
drivers/i2c/Makefile | 2 +-
drivers/i2c/i2c-core-base.c | 574 -
drivers/i2c/i2c-core-smbus.c | 594 +++
include/trace/events/i2c.h | 226 +--
include/trace/events/smbus.h | 249
6 files changed, 849 insertions(+), 799 deletions(-)
create mode 100644 drivers/i2c/i2c-core-smbus.c
create mode 100644 include/trace/events/smbus.h
diff --git a/Documentation/driver-api/i2c.rst b/Documentation/driver-api/i2c.rst
index 67366d9ff7303f..7582c079d74795 100644
--- a/Documentation/driver-api/i2c.rst
+++ b/Documentation/driver-api/i2c.rst
@@ -43,3 +43,6 @@ i2c_adapter devices which don't support those I2C operations.
.. kernel-doc:: drivers/i2c/i2c-core-base.c
:export:
+
+.. kernel-doc:: drivers/i2c/i2c-core-smbus.c
+ :export:
diff --git a/drivers/i2c/Makefile b/drivers/i2c/Makefile
index 6c54716e7f28ca..a6a90fe2db887a 100644
--- a/drivers/i2c/Makefile
+++ b/drivers/i2c/Makefile
@@ -4,7 +4,7 @@
obj-$(CONFIG_I2C_BOARDINFO)+= i2c-boardinfo.o
obj-$(CONFIG_I2C) += i2c-core.o
-i2c-core-objs := i2c-core-base.o
+i2c-core-objs := i2c-core-base.o i2c-core-smbus.o
i2c-core-$(CONFIG_I2C_SLAVE) += i2c-core-slave.o
obj-$(CONFIG_I2C_SMBUS)+= i2c-smbus.o
diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
index 88c0ca664a7b8f..70fc4624c69c25 100644
--- a/drivers/i2c/i2c-core-base.c
+++ b/drivers/i2c/i2c-core-base.c
@@ -14,9 +14,6 @@
/* - */
/* With some changes from Kyösti Mälkki .
- All SMBus-related things are written by Frodo Looijaard
- SMBus 2.0 support by Mark Studebaker and
- Jean Delvare
Mux support by Rodolfo Giometti and
Michael Lawnick
OF support is copyright (c) 2008 Jochen Friedrich
@@ -3155,577 +3152,6 @@ void i2c_put_adapter(struct i2c_adapter *adap)
}
EXPORT_SYMBOL(i2c_put_adapter);
-/* The SMBus parts */
-
-#define POLY(0x1070U << 3)
-static u8 crc8(u16 data)
-{
- int i;
-
- for (i = 0; i < 8; i++) {
- if (data & 0x8000)
- data = data ^ POLY;
- data = data << 1;
- }
- return (u8)(data >> 8);
-}
-
-/* Incremental CRC8 over count bytes in the array pointed to by p */
-static u8 i2c_smbus_pec(u8 crc, u8 *p, size_t count)
-{
- int i;
-
- for (i = 0; i < count; i++)
- crc = crc8((crc ^ p[i]) << 8);
- return crc;
-}
-
-/* Assume a 7-bit address, which is reasonable for SMBus */
-static u8 i2c_smbus_msg_pec(u8 pec, struct i2c_msg *msg)
-{
- /* The address will be sent first */
- u8 addr = i2c_8bit_addr_from_msg(msg);
- pec = i2c_smbus_pec(pec, &addr, 1);
-
- /* The data buffer follows */
- return i2c_smbus_pec(pec, msg->buf, msg->len);
-}
-
-/* Used for write only transactions */
-static inline void i2c_smbus_add_pec(struct i2c_msg *msg)
-{
- msg->buf[msg->len] = i2c_smbus_msg_pec(0, msg);
- msg->len++;
-}
-
-/* Return <0 on CRC error
- If there was a write before this read (most cases) we need to take the
- partial CRC from the write part into account.
- Note that this function does modify the message (we need to decrease the
- message length to hide the CRC byte from the caller). */
-static int i2c_smbus_check_pec(u8 cpec, struct i2c_msg *msg)
-{
- u8 rpec = msg->buf[--msg->len];
- cpec = i2c_smbus_msg_pec(cpec, msg);
-
- if (rpec != cpec) {
- pr_debug("Bad PEC 0x%02x vs. 0x%02x\n",
- rpec, cpec);
- return -EBADMSG;
- }
- return 0;
-}
-
-/**
- * i2c_smbus_read_byte - SMBus "receive byte" protocol
- * @client: Handle to slave device
- *
- * This executes the SMBus "receive byte" protocol, returning negative errno
- * else the byte received from the device.
- */
-s32 i2c_smbus_read_byte(const struct i2c_client *client)
-{
- union i2c_smbus_data data;
- int status;
-
- status = i2c_smbus_xfer(client->adapter, client->addr, client->flags,
- I2C_SMBUS_READ, 0,
- I2C_SMBUS_BYTE, &data);
- return (status < 0) ? status : data.byte;
-}
-EXPORT_SYMBOL(i2c_smbus_read_byte);
-
-/**
- * i2c_smbus_write_byte - SMBus "send byte" protocol
- * @client: Handle to slave device
- * @value: Byte to be sent
- *
- * This executes the SMBus "send byte" protocol, returning negative errno
- * else zero on success.
- *
[PATCH 6/8] docs: i2c: dev-interface: adapt to new filenames of the i2c core
The I2C core files were renamed, adapt the textfile to it. Signed-off-by: Wolfram Sang --- Documentation/i2c/dev-interface | 14 +++--- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/Documentation/i2c/dev-interface b/Documentation/i2c/dev-interface index bcf919d8625ceb..5ff19447ac4420 100644 --- a/Documentation/i2c/dev-interface +++ b/Documentation/i2c/dev-interface @@ -191,7 +191,7 @@ checking on future transactions.) 4* Other ioctl() calls are converted to in-kernel function calls by i2c-dev. Examples include I2C_FUNCS, which queries the I2C adapter functionality using i2c.h:i2c_get_functionality(), and I2C_SMBUS, which -performs an SMBus transaction using i2c-core.c:i2c_smbus_xfer(). +performs an SMBus transaction using i2c-core-smbus.c:i2c_smbus_xfer(). The i2c-dev driver is responsible for checking all the parameters that come from user-space for validity. After this point, there is no @@ -200,13 +200,13 @@ and calls that would have been performed by kernel I2C chip drivers directly. This means that I2C bus drivers don't need to implement anything special to support access from user-space. -5* These i2c-core.c/i2c.h functions are wrappers to the actual -implementation of your I2C bus driver. Each adapter must declare -callback functions implementing these standard calls. -i2c.h:i2c_get_functionality() calls i2c_adapter.algo->functionality(), -while i2c-core.c:i2c_smbus_xfer() calls either +5* These i2c.h functions are wrappers to the actual implementation of +your I2C bus driver. Each adapter must declare callback functions +implementing these standard calls. i2c.h:i2c_get_functionality() calls +i2c_adapter.algo->functionality(), while +i2c-core-smbus.c:i2c_smbus_xfer() calls either adapter.algo->smbus_xfer() if it is implemented, or if not, -i2c-core.c:i2c_smbus_xfer_emulated() which in turn calls +i2c-core-smbus.c:i2c_smbus_xfer_emulated() which in turn calls i2c_adapter.algo->master_xfer(). After your I2C bus driver has processed these requests, execution runs -- 2.11.0 -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] docs: driver-api: i2c: remove some outdated information
a) Linux can be an I2C slave meanwhile b) all drivers except one use the driver model currently Update the documentation. Signed-off-by: Wolfram Sang --- Documentation/driver-api/i2c.rst | 9 - 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/Documentation/driver-api/i2c.rst b/Documentation/driver-api/i2c.rst index f3939f7852bd59..0bf86a445d0135 100644 --- a/Documentation/driver-api/i2c.rst +++ b/Documentation/driver-api/i2c.rst @@ -13,8 +13,8 @@ I2C is a multi-master bus; open drain signaling is used to arbitrate between masters, as well as to handshake and to synchronize clocks from slower clients. -The Linux I2C programming interfaces support only the master side of bus -interactions, not the slave side. The programming interface is +The Linux I2C programming interfaces support the master side of bus +interactions and the slave side. The programming interface is structured around two kinds of driver, and two kinds of device. An I2C "Adapter Driver" abstracts the controller hardware; it binds to a physical device (perhaps a PCI device or platform_device) and exposes a @@ -22,9 +22,8 @@ physical device (perhaps a PCI device or platform_device) and exposes a I2C bus segment it manages. On each I2C bus segment will be I2C devices represented by a :c:type:`struct i2c_client `. Those devices will be bound to a :c:type:`struct i2c_driver -`, which should follow the standard Linux driver -model. (At this writing, a legacy model is more widely used.) There are -functions to perform various I2C protocol operations; at this writing +`, which should follow the standard Linux driver model. There +are functions to perform various I2C protocol operations; at this writing all such functions are usable only from task context. The System Management Bus (SMBus) is a sibling protocol. Most SMBus -- 2.11.0 -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/5] hwmon: move include files out of include/linux/i2c
Hi Guenter, > > Note that some files don't seem to have upstream > > users in board code, so they maybe could even be removed? I didn't check for > > While I understand where you are coming from, I am not typically that > aggressive. > Such removals force vendors who are not really forthcoming with upstreaming to > deviate even further from upstream. It makes them even less likely to submit > their > code upstream, and it may result in enforcing their belief that upstream > doesn't > really care about vendors struggling to release boards and systems to their > customers. I clearly see your point. I meant more the case where platform_data became cruft because platforms moved to DT. I agree this is not so much the case for HWMON but it was true for the I2C master driver where I could remove the platform data and could save the include file and some code. That was just a nice cleanup. > >I prefer the series to go upstream via the subsystem tree; if you prefer > >that I > >take it via I2C, just let me know. > > > Series applied to hwmon-next. Super, thanks! Regards, Wolfram signature.asc Description: PGP signature
[PATCH 1/5] hwmon: ads1015: move header file out of I2C realm
include/linux/i2c is not for client devices. Move the header file to a
more appropriate location.
Signed-off-by: Wolfram Sang
---
Documentation/hwmon/ads1015| 2 +-
MAINTAINERS| 2 +-
drivers/hwmon/ads1015.c| 2 +-
drivers/iio/adc/ti-ads1015.c | 2 +-
include/linux/{i2c => platform_data}/ads1015.h | 0
5 files changed, 4 insertions(+), 4 deletions(-)
rename include/linux/{i2c => platform_data}/ads1015.h (100%)
diff --git a/Documentation/hwmon/ads1015 b/Documentation/hwmon/ads1015
index 063b80d857b1f8..02d2a459385f39 100644
--- a/Documentation/hwmon/ads1015
+++ b/Documentation/hwmon/ads1015
@@ -40,7 +40,7 @@ By default all inputs are exported.
Platform Data
-
-In linux/i2c/ads1015.h platform data is defined, channel_data contains
+In linux/platform_data/ads1015.h platform data is defined, channel_data
contains
configuration data for the used input combinations:
- pga is the programmable gain amplifier (values are full scale)
0: +/- 6.144 V
diff --git a/MAINTAINERS b/MAINTAINERS
index 9e984645c4b08b..33541336258e77 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -478,7 +478,7 @@ L: linux-hw...@vger.kernel.org
S: Maintained
F: Documentation/hwmon/ads1015
F: drivers/hwmon/ads1015.c
-F: include/linux/i2c/ads1015.h
+F: include/linux/platform_data/ads1015.h
ADT746X FAN DRIVER
M: Colin Leroy
diff --git a/drivers/hwmon/ads1015.c b/drivers/hwmon/ads1015.c
index 5140c27d16dd03..357b4260716404 100644
--- a/drivers/hwmon/ads1015.c
+++ b/drivers/hwmon/ads1015.c
@@ -34,7 +34,7 @@
#include
#include
-#include
+#include
/* ADS1015 registers */
enum {
diff --git a/drivers/iio/adc/ti-ads1015.c b/drivers/iio/adc/ti-ads1015.c
index f76d979fb7e86e..884b8e461b1755 100644
--- a/drivers/iio/adc/ti-ads1015.c
+++ b/drivers/iio/adc/ti-ads1015.c
@@ -23,7 +23,7 @@
#include
#include
-#include
+#include
#include
#include
diff --git a/include/linux/i2c/ads1015.h b/include/linux/platform_data/ads1015.h
similarity index 100%
rename from include/linux/i2c/ads1015.h
rename to include/linux/platform_data/ads1015.h
--
2.11.0
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/5] hwmon: move include files out of include/linux/i2c
It doesn't make sense to use include/linux/i2c for client drivers which may in
fact rather be hwmon or input or whatever devices. As a result, I want to
deprecate include/linux/i2c for good. This series moves the include files to a
better location, largely include/platform_data because that is what most of the
moved include files contain. Note that some files don't seem to have upstream
users in board code, so they maybe could even be removed? I didn't check for
that now, but I did it for one i2c master driver recently. So, it may be
possible. pmbus.h got moved just one layer upwards, see the patch description
there.
I prefer the series to go upstream via the subsystem tree; if you prefer that I
take it via I2C, just let me know.
No runtime testing because of no HW, but buildbot is happy with this series at
least. A branch can be found here:
git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git i2c/platform_data
Thanks and kind regards,
Wolfram
Wolfram Sang (5):
hwmon: ads1015: move header file out of I2C realm
hwmon: ds620: move header file out of I2C realm
hwmon: ltc4245: move header file out of I2C realm
hwmon: max6639: move header file out of I2C realm
hwmon: pmbus: move header file out of I2C realm
Documentation/hwmon/ads1015| 2 +-
Documentation/hwmon/ltc4245| 2 +-
Documentation/hwmon/pmbus-core | 2 +-
MAINTAINERS| 4 ++--
drivers/hwmon/ads1015.c| 2 +-
drivers/hwmon/ds620.c | 2 +-
drivers/hwmon/ltc4245.c| 2 +-
drivers/hwmon/max6639.c| 2 +-
drivers/hwmon/pmbus/pmbus.c| 2 +-
drivers/hwmon/pmbus/pmbus_core.c | 2 +-
drivers/hwmon/pmbus/ucd9000.c | 2 +-
drivers/hwmon/pmbus/ucd9200.c | 2 +-
drivers/iio/adc/ti-ads1015.c | 2 +-
include/linux/{i2c => platform_data}/ads1015.h | 0
include/linux/{i2c => platform_data}/ds620.h | 0
include/linux/{i2c => platform_data}/ltc4245.h | 0
include/linux/{i2c => platform_data}/max6639.h | 0
include/linux/{i2c => }/pmbus.h| 0
18 files changed, 14 insertions(+), 14 deletions(-)
rename include/linux/{i2c => platform_data}/ads1015.h (100%)
rename include/linux/{i2c => platform_data}/ds620.h (100%)
rename include/linux/{i2c => platform_data}/ltc4245.h (100%)
rename include/linux/{i2c => platform_data}/max6639.h (100%)
rename include/linux/{i2c => }/pmbus.h (100%)
--
2.11.0
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/5] hwmon: ltc4245: move header file out of I2C realm
include/linux/i2c is not for client devices. Move the header file to a
more appropriate location.
Signed-off-by: Wolfram Sang
---
Documentation/hwmon/ltc4245| 2 +-
drivers/hwmon/ltc4245.c| 2 +-
include/linux/{i2c => platform_data}/ltc4245.h | 0
3 files changed, 2 insertions(+), 2 deletions(-)
rename include/linux/{i2c => platform_data}/ltc4245.h (100%)
diff --git a/Documentation/hwmon/ltc4245 b/Documentation/hwmon/ltc4245
index b478b086496586..4ca7a9da09f918 100644
--- a/Documentation/hwmon/ltc4245
+++ b/Documentation/hwmon/ltc4245
@@ -96,7 +96,7 @@ slowly, -EAGAIN will be returned when you read the sysfs
attribute containing
the sensor reading.
The LTC4245 chip can be configured to sample all GPIO pins with two methods:
-1) platform data -- see include/linux/i2c/ltc4245.h
+1) platform data -- see include/linux/platform_data/ltc4245.h
2) OF device tree -- add the "ltc4245,use-extra-gpios" property to each chip
The default mode of operation is to sample a single GPIO pin.
diff --git a/drivers/hwmon/ltc4245.c b/drivers/hwmon/ltc4245.c
index 4680d89556ce80..082f0a0bd8a0f1 100644
--- a/drivers/hwmon/ltc4245.c
+++ b/drivers/hwmon/ltc4245.c
@@ -23,7 +23,7 @@
#include
#include
#include
-#include
+#include
/* Here are names of the chip's registers (a.k.a. commands) */
enum ltc4245_cmd {
diff --git a/include/linux/i2c/ltc4245.h b/include/linux/platform_data/ltc4245.h
similarity index 100%
rename from include/linux/i2c/ltc4245.h
rename to include/linux/platform_data/ltc4245.h
--
2.11.0
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 5/5] hwmon: pmbus: move header file out of I2C realm
include/linux/i2c is not for client devices. Move the header file to a
more appropriate location.
Signed-off-by: Wolfram Sang
---
I decided to not move it to 'platform_data' but just one level up because
'pmbus.h' sounds pretty generic to me like 'i2c.h'. And it might contain
different stuff than platform data somewhen? Let me know if you think
different. Thanks!
Documentation/hwmon/pmbus-core | 2 +-
MAINTAINERS | 2 +-
drivers/hwmon/pmbus/pmbus.c | 2 +-
drivers/hwmon/pmbus/pmbus_core.c | 2 +-
drivers/hwmon/pmbus/ucd9000.c| 2 +-
drivers/hwmon/pmbus/ucd9200.c| 2 +-
include/linux/{i2c => }/pmbus.h | 0
7 files changed, 6 insertions(+), 6 deletions(-)
rename include/linux/{i2c => }/pmbus.h (100%)
diff --git a/Documentation/hwmon/pmbus-core b/Documentation/hwmon/pmbus-core
index 31e4720fed18c7..8ed10e9ddfb589 100644
--- a/Documentation/hwmon/pmbus-core
+++ b/Documentation/hwmon/pmbus-core
@@ -253,7 +253,7 @@ Specifically, it provides the following information.
PMBus driver platform data
==
-PMBus platform data is defined in include/linux/i2c/pmbus.h. Platform data
+PMBus platform data is defined in include/linux/pmbus.h. Platform data
currently only provides a flag field with a single bit used.
#define PMBUS_SKIP_STATUS_CHECK (1 << 0)
diff --git a/MAINTAINERS b/MAINTAINERS
index 33541336258e77..259cf67ac17067 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10155,7 +10155,7 @@ T: git
git://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git
S: Maintained
F: Documentation/hwmon/pmbus
F: drivers/hwmon/pmbus/
-F: include/linux/i2c/pmbus.h
+F: include/linux/pmbus.h
PMC SIERRA MaxRAID DRIVER
L: linux-s...@vger.kernel.org
diff --git a/drivers/hwmon/pmbus/pmbus.c b/drivers/hwmon/pmbus/pmbus.c
index 44ca8a94873d62..7718e58dbda543 100644
--- a/drivers/hwmon/pmbus/pmbus.c
+++ b/drivers/hwmon/pmbus/pmbus.c
@@ -25,7 +25,7 @@
#include
#include
#include
-#include
+#include
#include "pmbus.h"
/*
diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
index ba59eaef2e075a..f1eff6b6c79826 100644
--- a/drivers/hwmon/pmbus/pmbus_core.c
+++ b/drivers/hwmon/pmbus/pmbus_core.c
@@ -28,7 +28,7 @@
#include
#include
#include
-#include
+#include
#include
#include
#include "pmbus.h"
diff --git a/drivers/hwmon/pmbus/ucd9000.c b/drivers/hwmon/pmbus/ucd9000.c
index 3518f0c0893447..b74dbeca2e8d89 100644
--- a/drivers/hwmon/pmbus/ucd9000.c
+++ b/drivers/hwmon/pmbus/ucd9000.c
@@ -26,7 +26,7 @@
#include
#include
#include
-#include
+#include
#include "pmbus.h"
enum chips { ucd9000, ucd90120, ucd90124, ucd90160, ucd9090, ucd90910 };
diff --git a/drivers/hwmon/pmbus/ucd9200.c b/drivers/hwmon/pmbus/ucd9200.c
index a8712c5ded4e93..3ed94585837a9d 100644
--- a/drivers/hwmon/pmbus/ucd9200.c
+++ b/drivers/hwmon/pmbus/ucd9200.c
@@ -25,7 +25,7 @@
#include
#include
#include
-#include
+#include
#include "pmbus.h"
#define UCD9200_PHASE_INFO 0xd2
diff --git a/include/linux/i2c/pmbus.h b/include/linux/pmbus.h
similarity index 100%
rename from include/linux/i2c/pmbus.h
rename to include/linux/pmbus.h
--
2.11.0
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v9 00/10] mux controller abstraction and iio/i2c muxes
> And conflicts -- if they show up -- will probably be trivial given the > nature of the series. Famous last words... Heh. I agree, though :) signature.asc Description: PGP signature
Re: [PATCH v9 00/10] mux controller abstraction and iio/i2c muxes
> Jonathan, Wolfram, do you have any preferences on how this should be > coordinated regarding the new iio and i2c drivers (and iio changes)? You got the acks, all is fine, I think. > My plan is to at some point declare the branch immutable. Then both of > you can pull it in. Or? Yup, sounds good. > But now that I think about it I wonder what the point is of having > Greg pull it in also? Sure, third time's a charm and all that, but... AFAIU, Greg will be your "upstream", so he should definately pull the branch. I will just pull it in to avoid merge conflicts in my tree. Or did I misunderstand your question? signature.asc Description: PGP signature
Re: [PATCH] i2c: i2c-mux-gpio: rename i2c-gpio-mux to i2c-mux-gpio
On Tue, Feb 07, 2017 at 10:41:55PM +0100, Peter Rosin wrote:
> The rename did the wrong thing for this documentation file all those
> years ago. Fix that as well as the neglected rename of the platform
> data structure.
>
> Fixes: e7065e20d9a6 ("i2c: Rename last mux driver to standard pattern")
> Signed-off-by: Peter Rosin
Applied to for-next, thanks!
signature.asc
Description: PGP signature
Re: [PATCH linux v2 0/6] drivers: hwmon: Add On-Chip Controller driver
> This patchset adds a hwmon driver to support the OCC (On-Chip Controller) > on the IBM POWER8 and POWER9 processors, from a BMC (Baseboard Management > Controller). The OCC is an embedded processor that provides real time > power and thermal monitoring. Please don't cc the I2C list for I2C client drivers unless you have a specific question about the I2C framework. Same as you usually don't add the PCI list for every PCI (client) card. signature.asc Description: PGP signature
Re: [PATCH v7 00/12] mux controller abstraction and iio/i2c muxes
Hi peda, > One thing that I would like to do, but don't see a solution > for, is to move the mux control code that is present in > various drivers in drivers/i2c/muxes to this new minimalistic > muxing subsystem, thus converting all present i2c muxes (but > perhaps not gates and arbitrators) to be i2c-mux-simple muxes. In a few lines, what is preventing that? > I'm using an rwsem to lock a mux, but that isn't really a > perfect fit. Is there a better locking primitive that I don't > know about that fits better? I had a mutex at one point, but > that didn't allow any concurrent accesses at all. At least > the rwsem allows concurrent access as long as all users > agree on the mux state, but I suspect that the rwsem will > degrade to the mutex situation pretty quickly if there is > any contention. Maybe ask this question in a seperate email thread on lkml cc-ing the locking gurus (with a pointer to this thread)? > Also, the "mux" name feels a bit ambitious, there are many muxes > in the world, and this tiny bit of code is probably not good > enough to be a nice fit for all... "... and it probably never will support anything other than AT-harddisks, as that's all I have..." ;)) Thanks for this work! Wolfram signature.asc Description: PGP signature
Re: [PATCH v7 10/12] i2c: i2c-mux-simple: new driver
On Wed, Jan 04, 2017 at 01:16:25PM +0100, Peter Rosin wrote: > This is a generic simple i2c mux that uses the generic multiplexer > subsystem to do the muxing. > > The user can select if the mux is to be mux-locked and parent-locked > as described in Documentation/i2c/i2c-topology. > > Acked-by: Jonathan Cameron > Signed-off-by: Peter Rosin Acked-by: Wolfram Sang signature.asc Description: PGP signature
Re: [PATCH] i2c: i2c-topology: fix minor whitespace nit
On Thu, Nov 10, 2016 at 03:03:21PM +0100, Peter Rosin wrote: > Signed-off-by: Peter Rosin Applied to for-current, thanks! signature.asc Description: PGP signature
Re: [PATCH] mmc: core: Extend sysfs with OCR register
> Registers CID and CSD are already exported through sysfs so let's make > this interface complete by adding missing OCR register. This sentence was missing for me, thanks. > Do I need to send v2 with updated change log? Ulf will tell us. signature.asc Description: PGP signature
Re: [PATCH] mmc: core: Extend sysfs with OCR register
Bojan, On Mon, Jul 04, 2016 at 01:56:55PM +0200, Bojan Prtvar wrote: > Make operation conditions register (OCR) easily accessible from user space. > > Signed-off-by: Bojan Prtvar You described "what" above. Can you add the "why", too? Regards, Wolfram signature.asc Description: PGP signature
Re: [PATCH v8 3/4] i2c: i801: add support of Host Notify
On Thu, Jun 23, 2016 at 01:55:52PM -0700, Dmitry Torokhov wrote: > On Thu, Jun 16, 2016 at 08:09:42AM +0200, Wolfram Sang wrote: > > > > - removed the .resume hook as upstream changed suspend/resume hooks and > > > > there > > > > is no need in the end to re-enable host notify on resume (tested on > > > > Lenovo > > > > t440 and t450). > > > > > > Actually, this hook seemed to be required on the Lenovo T440 (Haswell) > > > but not on the T450 (Broadwell) laptop I have now here. > > > > > > Wolfram, I can resend the whole series or a follow-up patch to re-enable > > > after resume Host Notify. How do you prefer I deal with that? > > > > That depends a little how we want to handle patch 4. I am going to apply > > patches 1+2 today to my tree. Then you can just resend patch 3 which I > > hope will get some review soon, but I will pick it up for 4.8 later > > nonetheless. If it is not causing too much dependency hassle, I'd prefer > > that patch 4 goes via Dmitry's input tree. > > Any chance I could get a stable branch with these 2 patches based on 4.6 > so that I can pull it and merge the #4? This way we do not need to wait > for 2 releases to merge everything... Sure. git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git i2c/host-notify Thanks, Wolfram signature.asc Description: PGP signature
Re: [PATCH v8 1/4] i2c: add a protocol parameter to the alert callback
On Thu, Jun 09, 2016 at 04:53:47PM +0200, Benjamin Tissoires wrote: > .alert() is meant to be generic, but there is currently no way > for the device driver to know which protocol generated the alert. > Add a parameter in .alert() to help the device driver to understand > what is given in data. > > This patch is required to have the support of SMBus Host Notify protocol > through .alert(). > > Tested-by: Andrew Duggan > For hwmon: > Acked-by: Guenter Roeck > For IPMI: > Acked-by: Corey Minyard > Signed-off-by: Benjamin Tissoires Applied to for-next, thanks! signature.asc Description: PGP signature
Re: [PATCH v8 2/4] i2c-smbus: add SMBus Host Notify support
On Thu, Jun 09, 2016 at 04:53:48PM +0200, Benjamin Tissoires wrote: > SMBus Host Notify allows a slave device to act as a master on a bus to > notify the host of an interrupt. On Intel chipsets, the functionality > is directly implemented in the firmware. We just need to export a > function to call .alert() on the proper device driver. > > i2c_handle_smbus_host_notify() behaves like i2c_handle_smbus_alert(). > When called, it schedules a task that will be able to sleep to go through > the list of devices attached to the adapter. > > The current implementation allows one Host Notification to be scheduled > while an other is running. > > Tested-by: Andrew Duggan > Signed-off-by: Benjamin Tissoires Applied to for-next, thanks! signature.asc Description: PGP signature
Re: [PATCH v8 3/4] i2c: i801: add support of Host Notify
> (i2c-i801) can be carried over through the input tree. So as long as > you don't need to have a new feature without users for a short period > of time, that's fine by me :) That's fine. I have extremly high trust that the user of the feature will be added soon ;) signature.asc Description: PGP signature
Re: [PATCH v8 3/4] i2c: i801: add support of Host Notify
> > - removed the .resume hook as upstream changed suspend/resume hooks and > > there > > is no need in the end to re-enable host notify on resume (tested on Lenovo > > t440 and t450). > > Actually, this hook seemed to be required on the Lenovo T440 (Haswell) > but not on the T450 (Broadwell) laptop I have now here. > > Wolfram, I can resend the whole series or a follow-up patch to re-enable > after resume Host Notify. How do you prefer I deal with that? That depends a little how we want to handle patch 4. I am going to apply patches 1+2 today to my tree. Then you can just resend patch 3 which I hope will get some review soon, but I will pick it up for 4.8 later nonetheless. If it is not causing too much dependency hassle, I'd prefer that patch 4 goes via Dmitry's input tree. signature.asc Description: PGP signature
Re: [PATCH v7 0/4] i2c-smbus: add support for HOST NOTIFY
> OK. I'll try to fetch those pending patches on patchwork and see how the > merge would behave. Thanks. If you have time for a bit of a reviewing eye on them, this would also be much appreciated :) signature.asc Description: PGP signature
Re: [PATCH v7 3/4] i2c: i801: add support of Host Notify
On Tue, May 31, 2016 at 12:03:04PM +0200, Benjamin Tissoires wrote: > The i801 chip can handle the Host Notify feature since ICH 3 as mentioned > in > http://www.intel.com/content/dam/doc/datasheet/82801ca-io-controller-hub-3-datasheet.pdf > > Enable the functionality unconditionally and propagate the alert > on each notification. > > With a T440s and a Synaptics touchpad that implements Host Notify, the > payload data is always 0x, so I am not sure if the device actually > sends the payload or if there is a problem regarding the implementation. > > Tested-by: Andrew Duggan > Signed-off-by: Benjamin Tissoires Did some high level review. Did not dig into datasheets. Acked-by: Wolfram Sang signature.asc Description: PGP signature
Re: [PATCH v7 2/4] i2c-smbus: add SMBus Host Notify support
> diff --git a/Documentation/i2c/smbus-protocol
> b/Documentation/i2c/smbus-protocol
> index 6012b12..4e07848 100644
> --- a/Documentation/i2c/smbus-protocol
> +++ b/Documentation/i2c/smbus-protocol
> @@ -199,6 +199,9 @@ alerting device's address.
>
> [S] [HostAddr] [Wr] A [DevAddr] A [DataLow] A [DataHigh] A [P]
>
> +I2C drivers for devices which can trigger SMBus Host Notify should implement
> +the optional alert() callback.
> +
I'd like a similar "This is implemented the following way in the Linux
kernel:" paragraph here as in the alert section. The item what bus
drivers should do is missing.
> + if (host_notify->pending) {
> + spin_unlock_irqrestore(&host_notify->lock, flags);
> + dev_warn(&adapter->dev, "Host Notify already scheduled.\n");
> + return -EFAULT;
Very minor nit: -EBUSY?
> +#if defined(CONFIG_I2C_SMBUS) || defined(CONFIG_I2C_SMBUS_MODULE)
IS_ENABLED()?
signature.asc
Description: PGP signature
Re: [PATCH v7 1/4] i2c: add a protocol parameter to the alert callback
On Tue, May 31, 2016 at 12:03:02PM +0200, Benjamin Tissoires wrote: > .alert() is meant to be generic, but there is currently no way > for the device driver to know which protocol generated the alert. > Add a parameter in .alert() to help the device driver to understand > what is given in data. > > This patch is required to have the support of SMBus Host Notify protocol > through .alert(). > > Tested-by: Andrew Duggan signature.asc Description: PGP signature
Re: [PATCH v7 0/4] i2c-smbus: add support for HOST NOTIFY
Hi Benjamin, > this is mostly a resubmission of the v6 with the acks, tested-by and few typos > here and there. I actually reviewed v6 but got an NMI so writing the mails fell through the cracks :( Sorry about that! Good news is that the code is fine from my point of view, some documentation updates I'd request. After updating those, I will pick up the core patches right away. Note that while the i801 patch looks good to me, I need Jean's review here. There are other patches pending for i801, that needs to be coordinated by him. Thanks, Wolfram signature.asc Description: PGP signature
Re: [PATCH v9 0/9] i2c mux cleanup and locking update
On Wed, May 04, 2016 at 10:15:26PM +0200, Peter Rosin wrote: > Hi! > > I have a pair of boards with this i2c topology: > >GPIO ---| -- BAT1 > | v / >I2C -+--B---+ MUX > | \ >EEPROM -- BAT2 > > (B denotes the boundary between the boards) > > The problem with this is that the GPIO controller sits on the same i2c bus > that it MUXes. For pca954x devices this is worked around by using unlocked > transfers when updating the MUX. I have no such luck as the GPIO is a general > purpose IO expander and the MUX is just a random bidirectional MUX, unaware > of the fact that it is muxing an i2c bus. Extending unlocked transfers > into the GPIO subsystem is too ugly to even think about. But the general hw > approach is sane in my opinion, with the number of connections between the > two boards minimized. To put it plainly, I need support for it. > > So, I observe that while it is needed to have the i2c bus locked during the > actual MUX update in order to avoid random garbage on the slave side, it > is not strictly a must to have it locked over the whole sequence of a full > select-transfer-deselect operation. The MUX itself needs to be locked, so > transfers to clients behind the mux are serialized, and the MUX needs to be > stable during all i2c traffic (otherwise individual mux slave segments > might see garbage). > > This series accomplishes this by adding code to i2c-mux-gpio and > i2c-mux-pinctrl that determines if all involved devices used to update the > mux are controlled by the same root i2c adapter that is muxed. When this > is the case, the select-transfer-deselect operations should be locked > individually to avoid the deadlock. The i2c bus *is* still locked > during muxing, since the muxing happens as part of i2c transfers. This > is true even if the MUX is updated with several transfers to the GPIO (at > least as long as *all* MUX changes are using the i2c master bus). A lock > is added to i2c adapters that muxes on that adapter grab, so that transfers > through the muxes are serialized. > > Concerns: > - The locking is perhaps too complex? > - I worry about the priority inheritance aspect of the adapter lock. When > the transfers behind the mux are divided into select-transfer-deselect all > locked individually, low priority transfers get more chances to interfere > with high priority transfers. > - When doing an i2c_transfer() in_atomic() context or with irqs_disabled(), > there is a higher possibility that the mux is not returned to its idle > state after a failed (-EAGAIN) transfer due to trylock. > - Is the detection of i2c-controlled gpios and pinctrls sane (i.e. the > usage of the new i2c_root_adapter() function in 18/24)? > > The first half (patches 01-15 in v7) of what was originally part of this > series have already been scheduled for 4.6. So, this is the second half > (patches 16-24 in v7, patches 1-9 in v9). > > To summarize the series, there is some preparatory locking changes in > in 1/9 and the real meat is in 3/9. There is some documentation added in > 4/9 while 5/9 and after are cleanups to existing drivers utilizing > the new stuff. > > PS. needs a bunch of testing, I do not have access to all the involved hw. > > This second half of the series is planned to be merged with 4.7 and can > also be pulled from github, if that is preferred: > Applied all to for-next, thanks for keeping at it! signature.asc Description: PGP signature
Re: [PATCH v7 16/24] i2c: allow adapter drivers to override the adapter locking
> A question on best practices here... I already did a v8 (but only as > a branch) so I think this will be v9, bit that's a minor detail. The > real question is what I should do about patches 1-15 that are already > in next? Send them too? If not, should I send 16-24 with the same old > patch numbers or should they be numbered 1-9 now? And should such a > shortened series be rebased on 1-15 in next? > > Or does it not really matter? Easiest for me is: Send as v9, only the patches not yet applied, numbered from 1-9, based on my for-next. signature.asc Description: PGP signature
Re: [PATCH v7 16/24] i2c: allow adapter drivers to override the adapter locking
Hi Peter, thanks for the detailed explanation! > So maybe there should be only one flag, e.g. I2C_LOCK_ROOT_ADAPTER? > I.e. perhaps leave the future for later? I think this makes the current code easier understandable at this point, but I'll leave the decision to you. I am fine with both. Maybe a few words of explanation would be good if you want to keep both flags. > Hmmm, I just now realized that you were not really suggesting any > changes other than to the commit message. Oh well, I can perhaps > rephrase some of the above in the commit message if you think that > we should not unnecessarily touch the code at this point... Yes, updated commit description is enough for me now. If you want to change to one flag, we should do it incrementally. I think I can apply this as a fixup until around rc3 I'd say. > > I think this kerneldoc should be moved to i2c_lock_adapter and/or > > i2c_lock_bus() which are now in i2c.h. This is what users will use, not > > this static, adapter-specific implementation. I think it is enough to > > have a comment here explaining what is special in handling adapters. > > Yes, I was not really satisfied with having documentation on static > functions. But if I move it, there is no natural home for the current > i2c_trylock_adapter docs, and I'd hate killing documentation that > still applies. Do you have a suggestion? Maybe keep that one doc at > the static i2c_trylock_adapter for now and move it to ->trylock_bus > when someone decides to write kerneldoc for struct i2c_adapter? Well, because I think redundancy is acceptable when it comes to documentation, how about keeping the chunks you already have and copy an adapted one over to the functions in i2c.h? Regards, Wolfram signature.asc Description: PGP signature
Re: [PATCH v7 16/24] i2c: allow adapter drivers to override the adapter locking
> Yes, they look like reasonable complaints. Thanks for fixing them. I just sent out my latest comments and when you fix those and send V8, I'll apply that right away. I think we are safe to fix the rest incrementally if needed. Note that I didn't review the IIO and media patches, I trust the reviewers on those. Thanks for your work on this! I need a break now, this is mind-boggling... -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 18/24] i2c-mux: relax locking of the top i2c adapter during mux-locked muxing
> +static int i2c_mux_trylock_bus(struct i2c_adapter *adapter, int flags)
> +{
> + struct i2c_mux_priv *priv = adapter->algo_data;
> + struct i2c_adapter *parent = priv->muxc->parent;
> +
> + if (!rt_mutex_trylock(&parent->mux_lock))
> + return 0;
> + if (!(flags & I2C_LOCK_ADAPTER))
> + return 1;
> + if (parent->trylock_bus(parent, flags))
> + return 1;
> + rt_mutex_unlock(&parent->mux_lock);
> + return 0;
> +}
This function needs a few short comments why we can leave in this or
that state. Not everyone knows the exit values of trylock by heart and
then it can look a little confusing.
> static int i2c_parent_trylock_bus(struct i2c_adapter *adapter, int flags)
> @@ -111,7 +189,12 @@ static int i2c_parent_trylock_bus(struct i2c_adapter
> *adapter, int flags)
> struct i2c_mux_priv *priv = adapter->algo_data;
> struct i2c_adapter *parent = priv->muxc->parent;
>
> - return parent->trylock_bus(parent, flags);
> + if (!rt_mutex_trylock(&parent->mux_lock))
> + return 0;
> + if (parent->trylock_bus(parent, flags))
> + return 1;
> + rt_mutex_unlock(&parent->mux_lock);
> + return 0;
> }
Same comment as i2c_mux_trylock_bus.
> struct i2c_mux_core *i2c_mux_alloc(struct i2c_adapter *parent,
> struct device *dev, int max_adapters,
> @@ -140,6 +250,8 @@ struct i2c_mux_core *i2c_mux_alloc(struct i2c_adapter
> *parent,
>
> muxc->parent = parent;
> muxc->dev = dev;
> + if (flags & I2C_MUX_LOCKED)
> + muxc->mux_locked = 1;
s/1/true/;
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 16/24] i2c: allow adapter drivers to override the adapter locking
On Wed, Apr 20, 2016 at 05:17:56PM +0200, Peter Rosin wrote: > Add i2c_lock_bus() and i2c_unlock_bus(), which call the new lock_bus and > unlock_bus ops in the adapter. These funcs/ops take an additional flags > argument that indicates for what purpose the adapter is locked. > > There are two flags, I2C_LOCK_ADAPTER and I2C_LOCK_SEGMENT, but they are > both implemented the same. For now. Locking the adapter means that the > whole bus is locked, locking the segment means that only the current bus > segment is locked (i.e. i2c traffic on the parent side of mux is still > allowed even if the child side of the mux is locked. > > Also support a trylock_bus op (but no function to call it, as it is not > expected to be needed outside of the i2c core). > > Implement i2c_lock_adapter/i2c_unlock_adapter in terms of the new locking > scheme (i.e. lock with the I2C_LOCK_ADAPTER flag). > > Annotate some of the locking with explicit I2C_LOCK_SEGMENT flags. Can you explain a little why it is SEGMENT and not ADAPTER here? That probably makes it easier to get into this patch. And to double check my understanding: I was surprised to not see any i2c_lock_adapter() or I2C_LOCK_ADAPTER in action. This is because muxes call I2C_LOCK_SEGMENT on their parent which in case of the parent being the root adapter is essentially the same as I2C_LOCK_ADAPTER. Correct? > > Signed-off-by: Peter Rosin > --- > drivers/i2c/i2c-core.c | 46 -- > include/linux/i2c.h| 28 ++-- > 2 files changed, 54 insertions(+), 20 deletions(-) > > diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c > index 0f2f8484e8ec..21f46d011c33 100644 > --- a/drivers/i2c/i2c-core.c > +++ b/drivers/i2c/i2c-core.c > @@ -960,10 +960,12 @@ static int i2c_check_addr_busy(struct i2c_adapter > *adapter, int addr) > } > > /** > - * i2c_lock_adapter - Get exclusive access to an I2C bus segment > + * i2c_adapter_lock_bus - Get exclusive access to an I2C bus segment > * @adapter: Target I2C bus segment > + * @flags: I2C_LOCK_ADAPTER locks the root i2c adapter, I2C_LOCK_SEGMENT > + * locks only this branch in the adapter tree > */ I think this kerneldoc should be moved to i2c_lock_adapter and/or i2c_lock_bus() which are now in i2c.h. This is what users will use, not this static, adapter-specific implementation. I think it is enough to have a comment here explaining what is special in handling adapters. Thanks, Wolfram -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 16/24] i2c: allow adapter drivers to override the adapter locking
> Yes, obviously... I'll make that change locally and wait for the rest. Another nit: You could use '--strict' with checkpatch and see if you want to fix the issues reported. I am not keen on those (except for 'space around operators'), it's a matter of taste I guess, but maybe you like some of the suggestions. Thanks, Wolfram signature.asc Description: PGP signature
Re: [PATCH v7 22/24] [media] rtl2832: change the i2c gate to be mux-locked
> So, I think all is ok, or do you need more than Tested-by? I think this will do. Thanks! signature.asc Description: PGP signature
Re: [PATCH v7 22/24] [media] rtl2832: change the i2c gate to be mux-locked
On Wed, Apr 20, 2016 at 05:18:02PM +0200, Peter Rosin wrote: > The root i2c adapter lock is then no longer held by the i2c mux during > accesses behind the i2c gate, and such accesses need to take that lock > just like any other ordinary i2c accesses do. > > So, declare the i2c gate mux-locked, and zap the regmap overrides > that makes the i2c accesses unlocked and use plain old regmap > accesses. This also removes the need for the regmap wrappers used by > rtl2832_sdr, so deconvolute the code further and provide the regmap > handle directly instead of the wrapper functions. > > Signed-off-by: Peter Rosin Antti, I'd need some tag from you since this is not the i2c realm. signature.asc Description: PGP signature
Re: [PATCH v7 16/24] i2c: allow adapter drivers to override the adapter locking
On Wed, Apr 20, 2016 at 05:17:56PM +0200, Peter Rosin wrote: > Add i2c_lock_bus() and i2c_unlock_bus(), which call the new lock_bus and > unlock_bus ops in the adapter. These funcs/ops take an additional flags > argument that indicates for what purpose the adapter is locked. > > There are two flags, I2C_LOCK_ADAPTER and I2C_LOCK_SEGMENT, but they are > both implemented the same. For now. Locking the adapter means that the > whole bus is locked, locking the segment means that only the current bus > segment is locked (i.e. i2c traffic on the parent side of mux is still > allowed even if the child side of the mux is locked. > > Also support a trylock_bus op (but no function to call it, as it is not > expected to be needed outside of the i2c core). > > Implement i2c_lock_adapter/i2c_unlock_adapter in terms of the new locking > scheme (i.e. lock with the I2C_LOCK_ADAPTER flag). > > Annotate some of the locking with explicit I2C_LOCK_SEGMENT flags. > > Signed-off-by: Peter Rosin Letting you know that I start reviewing the 2nd part of your series. Did the first glimpse today. Will hopefully do the in-depth part this weekend. One thing already: > +static void i2c_adapter_lock_bus(struct i2c_adapter *adapter, int flags) Shouldn't flags be unsigned? signature.asc Description: PGP signature
Re: [PATCH v7 00/24] i2c mux cleanup and locking update
> The problem with waiting until 4.8 with the rest of the series is that it > will likely go stale, e.g. patch 22 ([media] rtl2832: change the i2c gate > to be mux-locked) touches a ton of register accesses in that driver since > it removes a regmap wrapper that is rendered obsolete. Expecting that > patch to work for 4.8 is overly optimistic, and while patching things up Okay, that can be argued, I understand that. So, what about this suggestion: I pull in patches 1-15 today, and we schedule the rest of the patches for like next week or so. That still gives the first set of patches some time in linux-next for further exposure and testing whilst the whole series should arrive in 4.7. However, I need help with that. There are serious locking changes involved and ideally these patches are reviewed by multiple people, especially patches 16-19. I first want to say that the collaboration experience with this series was great so far, lots of testing and reporting back. Thanks for that already. Yet, if we want to have this in 4.7, this needs to be a group effort. So, if people interested could review even a little and report back this would be extremly helpful. > Third, should we deprecate the old i2c_add_mux_adapter, so that new > users do not crop up behind our backs in the transition? Or not bother? Usually it is fine to change in-kernel-APIs when you take care that all current users are converted. But I am also fine with being nice and keeping the old call around for a few cycles. It is your call. > Fourth, I forgot to change patch 8 (iio: imu: inv_mpu6050: convert to > use an explicit i2c mux core) to not change i2c_get_clientdata() -> > dev_get_drvdata() as requested by Jonathan Cameron. How should I handle > that? I'll pull in the first patches this eveneing. You can choose to send me an incremental patch or resend patch 8. I am fine with both, but it should appear on the mailing list somehow. > There are also some new Tested-by tags that I have added to my > local branch but have not pushed anywhere. I'm ready to push all that > to a new branch once you are ready to take it. For the patches 1-15, I am ready when you are :) Thanks, Wolfram signature.asc Description: PGP signature
Re: [PATCH v7 00/24] i2c mux cleanup and locking update
This was the diff of v6: > 32 files changed, 1277 insertions(+), 915 deletions(-) This is v7: > 32 files changed, 1225 insertions(+), 916 deletions(-) So, we gained a little overall. And while the individual drivers have a few lines more now, I still think it is more readable. So, thanks for doing that! I'll give people some time for testing. I'll have a further look, too. Hopefully, I can pick up patches 1-14 by the end of the week. Wolfram signature.asc Description: PGP signature
Re: [PATCH v6 01/24] i2c-mux: add common data for every i2c-mux instance
> > I'd suggest to rename 'adapters' into 'num_adapters' throughout this > > patch. I think it makes the code a lot easier to understand. > > Hmm, you mean just the variable names, right? And not function names > such as i2c_mux_reserve_(num_)adapters? Yes, only variable names. > > Despite that I wonder why not using some of the realloc functions, I > > When I wrote it, I found no devm_ version of realloc. I'm not finding > anything now either... Right, there is no devm_ version of it :( > > wonder even more if we couldn't supply num_adapters to i2c_mux_alloc() > > and reserve the memory statically. i2c busses are not > > dynamic/hot-pluggable so that should be good enough? > > Yes, that would work, but it would take some restructuring in some of > the drivers that currently don't know how many child adapters they > are going to need when they call i2c_mux_alloc. Which ones? > Because you thought about removing i2c_mux_reserve_adapters completely, > and not provide any means of adding more adapters than specified in > the i2c_mux_alloc call, right? Yes. I assumed I2C to be static enough that such information is known in advance. > > Ignoring the 80 char limit here makes the code more readable. > > That is only true if you actually have more than 80 characters, so I don't > agree. Are you adamant about it? (I'm not) No. Keep it if you prefer it. > >> +EXPORT_SYMBOL_GPL(i2c_mux_one_adapter); > > > > Are you sure the above function pays off? Its argument list is very > > complex and it doesn't save a lot of code. Having seperate calls is > > probably more understandable in drivers? Then again, I assume it makes > > the conversion of existing drivers easier. > > I added it in v4, you can check earlier versions if you like. Without > it most gate-muxes (i.e. typically the muxes in drivers/media) grew > since the i2c_add_mux_adapter call got replaced by two calls, i.e. > i2c_mux_alloc followed by i2c_max_add_adapter, and coupled with > error checks made it look more complex than before. So, this wasn't > much of a cleanup from the point of those drivers. Hmm, v3 didn't have the driver patches posted with it. Can you push it to your branch? I am also not too strong with this one, but having a look how it looks without would be nice. signature.asc Description: PGP signature
Re: [PATCH v6 01/24] i2c-mux: add common data for every i2c-mux instance
Hi Peter,
first high-level review:
> +int i2c_mux_reserve_adapters(struct i2c_mux_core *muxc, int adapters)
I'd suggest to rename 'adapters' into 'num_adapters' throughout this
patch. I think it makes the code a lot easier to understand.
> +{
> + struct i2c_adapter **adapter;
> +
> + if (adapters <= muxc->max_adapters)
> + return 0;
> +
> + adapter = devm_kmalloc_array(muxc->dev,
> + adapters, sizeof(*adapter),
> + GFP_KERNEL);
> + if (!adapter)
> + return -ENOMEM;
> +
> + if (muxc->adapter) {
> + memcpy(adapter, muxc->adapter,
> +muxc->max_adapters * sizeof(*adapter));
> + devm_kfree(muxc->dev, muxc->adapter);
> + }
> +
> + muxc->adapter = adapter;
> + muxc->max_adapters = adapters;
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(i2c_mux_reserve_adapters);
Despite that I wonder why not using some of the realloc functions, I
wonder even more if we couldn't supply num_adapters to i2c_mux_alloc()
and reserve the memory statically. i2c busses are not
dynamic/hot-pluggable so that should be good enough?
> - WARN(sysfs_create_link(&priv->adap.dev.kobj, &mux_dev->kobj,
> "mux_device"),
> -"can't create symlink to mux device\n");
> + WARN(sysfs_create_link(&priv->adap.dev.kobj, &muxc->dev->kobj,
> +"mux_device"),
Ignoring the 80 char limit here makes the code more readable.
> + "can't create symlink to mux device\n");
>
> snprintf(symlink_name, sizeof(symlink_name), "channel-%u", chan_id);
> - WARN(sysfs_create_link(&mux_dev->kobj, &priv->adap.dev.kobj,
> symlink_name),
> -"can't create symlink for channel %u\n",
> chan_id);
> + WARN(sysfs_create_link(&muxc->dev->kobj, &priv->adap.dev.kobj,
> +symlink_name),
ditto.
> + "can't create symlink for channel %u\n", chan_id);
> dev_info(&parent->dev, "Added multiplexed i2c bus %d\n",
>i2c_adapter_id(&priv->adap));
>
> - return &priv->adap;
> + muxc->adapter[muxc->adapters++] = &priv->adap;
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(i2c_mux_add_adapter);
> +
> +struct i2c_mux_core *i2c_mux_one_adapter(struct i2c_adapter *parent,
> + struct device *dev, int sizeof_priv,
> + u32 flags, u32 force_nr,
> + u32 chan_id, unsigned int class,
> + int (*select)(struct i2c_mux_core *,
> +u32),
ditto
> + int (*deselect)(struct i2c_mux_core *,
> + u32))
ditto
> +{
> + struct i2c_mux_core *muxc;
> + int ret;
> +
> + muxc = i2c_mux_alloc(parent, dev, sizeof_priv, flags, select, deselect);
> + if (!muxc)
> + return ERR_PTR(-ENOMEM);
> +
> + ret = i2c_mux_add_adapter(muxc, force_nr, chan_id, class);
> + if (ret) {
> + devm_kfree(dev, muxc);
> + return ERR_PTR(ret);
> + }
> +
> + return muxc;
> +}
> +EXPORT_SYMBOL_GPL(i2c_mux_one_adapter);
Are you sure the above function pays off? Its argument list is very
complex and it doesn't save a lot of code. Having seperate calls is
probably more understandable in drivers? Then again, I assume it makes
the conversion of existing drivers easier.
So much for now. Thanks!
Wolfram
signature.asc
Description: PGP signature
Re: [PATCH v6 00/24] i2c mux cleanup and locking update
> can obviously take forever. At the same time, many of the patches are kind > of mechanical, and feels rather safe. I agree about the mechanical stuff, thus my suggestion. We do what we can about testing and reviewing. And once it reaches linux-next (hopefully next week latest), test coverage will increase significantly and we can fix issues incrementally from there on. Same goes when it finally hits Linus' tree, coverage will increase more, but we should be really at a very sane level then ;) I will also pick up patch 15 (the removal) for 4.8. So we have the full 4.7 cycle to revert if something goes very wrong. > Maybe we should give Antti some more time to re-add his tested-by tags > on 9-12 before they are merged into non-rewritable branches? Yes. That would be great. I need to sync with the media maintainers anyhow. I'd like to push all the patches via my tree. Thanks, Wolfram signature.asc Description: PGP signature
Re: [PATCH v6 00/24] i2c mux cleanup and locking update
Hi Peter, > To summarize the series, there's some i2c-mux infrastructure cleanup work > first (I think that part stands by itself as desireable regardless), the > locking changes are in 16/24 and after with the real meat in 18/24. There > is some documentation added in 19/24 while 20/24 and after are cleanups to > existing drivers utilizing the new stuff. My idea is to review and pull in the infrastructure work for 4.7 and the locking changes to 4.8. This gives us one cycle to fix regressions (if any) in the infrastructure work first. Is that okay with you? Thanks, Wolfram signature.asc Description: PGP signature
Re: [PATCH] Doc: i2c: Fix typo in Documentation/i2c
On Tue, Feb 02, 2016 at 08:41:25PM +0900, Masanari Iida wrote: > This path fix spelling typos found in Documentation/i2c. > > Signed-off-by: Masanari Iida Probably too late already, but still: Acked-by: Wolfram Sang Thanks! signature.asc Description: Digital signature
