Re: [RESEND PATCH 0/9] eeprom: at24: at24cs series serial number read
>>>>> "Bartosz" == Bartosz Golaszewski writes: Hi, >> > I wanted to respond that this way we would not be protected from >> > concurrent accesses, but then I saw I didn't actually include any >> > locks in the serial read function - my bad. It needs to be fixed as >> > both memory blocks share the same address pointer. >> >> > I'll resend the series. >> >> But we're protected by the i2c bus lock, right? You do a single >> i2c_transfer to read the serial number. > Why the at24->lock then? I'm not sure. Wolfram? -- Bye, Peter Korsgaard -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RESEND PATCH 0/9] eeprom: at24: at24cs series serial number read
>>>>> "Bartosz" == Bartosz Golaszewski writes: >> As the serial number is available on a separate i2c address, wouldn't >> it be simpler to handle these as special (read only) device variants and >> instantiate E.G. a 24c64 (for the normal data) and a 24cs64 (for the >> serial)? >> > Hi Peter, > I wanted to respond that this way we would not be protected from > concurrent accesses, but then I saw I didn't actually include any > locks in the serial read function - my bad. It needs to be fixed as > both memory blocks share the same address pointer. > I'll resend the series. But we're protected by the i2c bus lock, right? You do a single i2c_transfer to read the serial number. -- Venlig hilsen, Peter Korsgaard -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RESEND PATCH 0/9] eeprom: at24: at24cs series serial number read
>>>>> "Bartosz" == Bartosz Golaszewski writes: > Chips from the at24cs EEPROM series have an additional read-only memory area > containing a factory pre-programmed serial number. In order to access it, a > dummy write must be executed before reading the serial number bytes. > This series adds support for reading the serial number through a sysfs > attribute. > While we're at it: some of the patches contain readability tweaks and code > organization fixes. > Tested with at24cs64 and at24cs02 chips (for both 16 and 8 bit address > pointers). As the serial number is available on a separate i2c address, wouldn't it be simpler to handle these as special (read only) device variants and instantiate E.G. a 24c64 (for the normal data) and a 24cs64 (for the serial)? -- Bye, Peter Korsgaard -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: multiple i2c-ocores adapters
>>>>> "York" == York Sun writes: Hi, >> I guess your problem is that the driver core is complaining about >> duplicate names for your platform devices (generated from the >> mfd_cell). Make sure you set the .id member to something unique. > What I got was > sysfs: cannot create duplicate filename '/bus/platform/devices/ocores-i2c' > I think it is caused by the i2c-ocores driver. Will dig deeper. Like I said, this sounds like you are trying to register multiple platform devices with the same name (ocores-i2c) and id == -1. Please make sure you use unique ids when you have more than 1 device instance on your system. -- Bye, Peter Korsgaard -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: multiple i2c-ocores adapters
>>>>> "York" == York Sun writes: > Peter, > I have a platform (FPGA) with multiple ocores i2c adapter. When I > register them using MFD framework, I got a message regarding > duplicating name for sysfs. I wonder if this driver (i2c-ocores.c) > only supports one adapter. I can try to fix it by adding a name string > into ocores_i2c_platform_data and allocate struct i2c_adapter on > demand. Am I on the right direction? I guess your problem is that the driver core is complaining about duplicate names for your platform devices (generated from the mfd_cell). Make sure you set the .id member to something unique. -- Bye, Peter Korsgaard -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [linux-sunxi] [PATCH] i2c: mv64xxx: The n clockdiv factor is 0 based on sunxi SoCs
>>>>> "Hans" == Hans de Goede writes: > According to the datasheets to n factor for dividing the tclk is s/to/the/ -- Bye, Peter Korsgaard -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] i2c-ocores: support big-endian register layout
>>>>> "Max" == Max Filippov writes: Hi, >> You should also document the big-endian property in >> Documentation/devicetree/bindings/i2c/i2c-ocores.txt, otherwise it looks >> good. > 'big-endian' and 'native-endian' are common properties documented > in the Documentation/devicetree/bindings/common-properties.txt Ok, then it looks good to me. -- Bye, Peter Korsgaard -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] i2c-ocores: support big-endian register layout
>>>>> "Max" == Max Filippov writes: > This allows using OpenCores I2C controller attached to its host in > native-endian mode with bi-endian CPUs. Example of such system is Xtensa > XTFPGA platform. > Signed-off-by: Max Filippov > --- > Changes v1->v2: > - expand changelog with motivation for the change. You should also document the big-endian property in Documentation/devicetree/bindings/i2c/i2c-ocores.txt, otherwise it looks good. With that added: Acked-by: Peter Korsgaard -- Bye, Peter Korsgaard -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] i2c: i2c-mux-gpio: remove error messages for probe deferrals
>>>>> "AS" == Alexander Sverdlin writes: Hi, AS> On 30/03/15 15:03, Ioan Nicu wrote: >> Probe deferral is not an error case. It happens only when >> the necessary dependencies are not there yet. >> >> The driver core is already printing a message when a driver >> requests probe deferral, so this can be traced in the logs >> without these error prints. >> >> This patch removes the error messages for these deferral cases. >> >> Signed-off-by: Ionut Nicu AS> Acked-by: Alexander Sverdlin Acked-by: Peter Korsgaard -- Sorry about disclaimer - It's out of my control. Bye, Peter Korsgaard This message is subject to the following terms and conditions: MAIL DISCLAIMER<http://www.barco.com/en/maildisclaimer> -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] i2c: i2c-mux-gpio: Change log level to debug for probe deferrals
>>>>> "IN" == Ioan Nicu writes: IN> Probe deferral is not an error case. It happens only when IN> the necessary dependencies are not there yet. IN> The driver core is already printing a message when a driver IN> requests probe deferral, so this can be traced in the logs IN> without these error prints. IN> This patch changes the error messages from these deferral cases IN> to debug messages. IN> Signed-off-by: Ionut Nicu Acked-by: Peter Korsgaard -- Sorry about disclaimer - It's out of my control. Bye, Peter Korsgaard This message is subject to the following terms and conditions: MAIL DISCLAIMER<http://www.barco.com/en/maildisclaimer> -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] i2c-ocores: add common clock support
>>>>> "Wolfram" == Wolfram Sang writes: >> The clock here is not the i2c bus clock, but the clock input of the > Yes, what I would expect from a clk-property :) >> controller. The function ocores_init initializes the prescaler register of >> the controller so that the bus clock equals 100kHz (internal clock >> runs at 500kHz): > 'clock-frequency' usually describes the I2C bus speed. So, for ocores, > it describes speed of the clock for the controller? That would be > ouch... Indeed :/ Looking back in the history, the device tree patch originally used a custom "clock_khz" property until some guy told him to use clock-frequency ;) https://lists.ozlabs.org/pipermail/devicetree-discuss/2010-November/003650.html As far as I can see I wasn't CC'ed on that patch. -- Bye, Peter Korsgaard -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] i2c-ocores: add common clock support
>>>>> "Wolfram" == Wolfram Sang writes:
>> @@ -320,9 +322,23 @@ static int ocores_i2c_of_probe(struct platform_device
>> *pdev,
>> }
>>
>> if (of_property_read_u32(np, "clock-frequency", &val)) {
>> - dev_err(&pdev->dev,
>> - "Missing required parameter 'clock-frequency'\n");
>> - return -ENODEV;
>> + struct clk *clk = devm_clk_get(&pdev->dev, NULL);
>> +
>> + if (!IS_ERR(clk)) {
>> + int ret = clk_prepare_enable(clk);
>> +
>> + if (ret) {
>> + dev_err(&pdev->dev,
>> + "clk_prepare_enable failed: %d\n", ret);
>> + return ret;
>> + }
>> + i2c->clk = clk;
>> + val = clk_get_rate(clk);
>> + } else {
>> + dev_err(&pdev->dev,
>> + "Missing required parameter
>> 'clock-frequency'\n");
>> + return -ENODEV;
>> + }
> Either NAK or I don't understand the logic here :) If a dts does NOT
> have the bus-speed set by 'clock-frequency', then we take the value of
> the clock assigned to this platform_device?
> The usual thing to do when 'clock-frequency' is not set is to default to
> 100kHz.
The confusion comes from the fact that the device tree bindings uses
clock-frequency for the clock frequency of the IP core and NOT for the
i2c bus frequency. This dates back to when the bindings where added
(049bb69d82e5f7f356) :/
The driver is currently hardcoded to use a 100KHz i2c bus clock (see
ocoores_init()).
--
Bye, Peter Korsgaard
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] i2c-ocores: add common clock support
>>>>> "Max" == Max Filippov writes: > Allow bus clock specification as a common clock handle. This makes this > controller easier to use in a setup based on common clock framework. > Signed-off-by: Max Filippov Looks sensible to me - Thanks. Acked-by: Peter Korsgaard -- Bye, Peter Korsgaard -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 09/16] i2c: i2c-ocores: Drop class based scanning to improve bootup time
>>>>> "Wolfram" == Wolfram Sang writes: > This driver has been flagged to drop class based instantiation. The removal > improves boot-up time and is unneeded for embedded controllers. Users have > been > warned to switch for some time now, so we can actually do the removal. Keep > the > DEPRECATED flag, so the core can inform users that the behaviour finally > changed now. After another transition period, this flag can go, too. > While we are here, remove the indentation for the array setup because > such things always break after some time. > Signed-off-by: Wolfram Sang Acked-by: Peter Korsgaard -- Bye, Peter Korsgaard -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2] i2c: ocores: Make of_device_id array const
>>>>> "Jingoo" == Jingoo Han writes: You seem to have forgotten to change subject, but I guess Wolfram can fix that when committing (E.G. s/ocores/busses/). > Make of_device_id array const, because all OF functions > handle it as const. > Signed-off-by: Jingoo Han > Acked-by: Peter Korsgaard > Acked-by: Maxime Coquelin > --- > Changes since v1: > - squashed all patches into a single patch > - added acked-by -- By, Peter Korsgaard -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/5] i2c: ocores: Make of_device_id array const
>>>>> "Jingoo" == Jingoo Han writes:
> Make of_device_id array const, because all OF functions
> handle it as const.
> Signed-off-by: Jingoo Han
Acked-by: Peter Korsgaard
> ---
> drivers/i2c/busses/i2c-ocores.c |2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
> diff --git a/drivers/i2c/busses/i2c-ocores.c
> b/drivers/i2c/busses/i2c-ocores.c
> index 1f6369f..0e10cc6 100644
> --- a/drivers/i2c/busses/i2c-ocores.c
> +++ b/drivers/i2c/busses/i2c-ocores.c
> @@ -250,7 +250,7 @@ static struct i2c_adapter ocores_adapter = {
> .algo = &ocores_algorithm,
> };
> -static struct of_device_id ocores_i2c_match[] = {
> +static const struct of_device_id ocores_i2c_match[] = {
> {
> .compatible = "opencores,i2c-ocores",
> .data = (void *)TYPE_OCORES,
> --
> 1.7.10.4
> --
> To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Bye, Peter Korsgaard
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 11/17] i2c: i2c-ocores: deprecate class based instantiation
>>>>> "Wolfram" == Wolfram Sang writes: > Warn users that class based instantiation is going away soon in favour > of more robust probing and faster bootup times. > Signed-off-by: Wolfram Sang > Cc: Peter Korsgaard > --- > This patch is a suggestion. Looking for an ack by someone who actually uses > the driver. It's been some time since I last used it, but I never needed the class based probing, so: Acked-by: Peter Korsgaard -- Bye, Peter Korsgaard -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] i2c-mux-gpio: use gpio_set_value_cansleep()
>>>>> "IN" == Ionut Nicu writes: IN> Some gpio chips may have get/set operations that IN> can sleep. gpio_set_value() only works for chips IN> which do not sleep, for the others we will get a IN> kernel warning. Using gpio_set_value_cansleep() IN> will work for both chips that do sleep and those IN> who don't. IN> Signed-off-by: Ionut Nicu IN> --- IN> drivers/i2c/muxes/i2c-mux-gpio.c |4 ++-- IN> 1 files changed, 2 insertions(+), 2 deletions(-) IN> diff --git a/drivers/i2c/muxes/i2c-mux-gpio.c b/drivers/i2c/muxes/i2c-mux-gpio.c IN> index a764da7..550e094 100644 IN> --- a/drivers/i2c/muxes/i2c-mux-gpio.c IN> +++ b/drivers/i2c/muxes/i2c-mux-gpio.c IN> @@ -30,8 +30,8 @@ static void i2c_mux_gpio_set(const struct gpiomux *mux, unsigned val) IN> int i; IN> for (i = 0; i < mux->data.n_gpios; i++) IN> - gpio_set_value(mux->gpio_base + mux->data.gpios[i], IN> - val & (1 << i)); IN> + gpio_set_value_cansleep(mux->gpio_base + mux->data.gpios[i], IN> + val & (1 << i)); The indentation of the 2nd line seems wrong (should match mux->gpio_base), otherwise it looks good: Acked-by: Peter Korsgaard -- Sorry about disclaimer - It's out of my control. Bye, Peter Korsgaard This message is subject to the following terms and conditions: MAIL DISCLAIMER<http://www.barco.com/en/maildisclaimer> -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] i2c-mux-gpio: use deferred probing
>>>>> "IN" == Ionut Nicu writes: IN> If the i2c-parent bus driver is not loaded, returning IN> -ENODEV will force people to unload and then reload the IN> module again to get it working. IN> Signed-off-by: Ionut Nicu Acked-by: Peter Korsgaard -- Sorry about disclaimer - It's out of my control. Bye, Peter Korsgaard This message is subject to the following terms and conditions: MAIL DISCLAIMER<http://www.barco.com/en/maildisclaimer> -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] i2c-mux-gpio: use deferred probing with the device tree
>>>>> "WS" == Wolfram Sang writes: WS> On Tue, Oct 08, 2013 at 03:51:50PM +0200, Ionut Nicu wrote: >> If the i2c-parent bus driver is not loaded, returning >> -EINVAL will force people to unload and then reload the >> module again to get it working. >> >> Signed-off-by: Ionut Nicu WS> Doesn't the non-DT case need fixing, too? Arguably yes. -- Bye, Peter Korsgaard This message is subject to the following terms and conditions: MAIL DISCLAIMER<http://www.barco.com/en/maildisclaimer> -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] i2c-mux-gpio: don't ignore of_get_named_gpio errors
>>>>> "IN" == Ionut Nicu writes: IN> of_get_named_gpio could return -E_PROBE_DEFER or another IN> error code. This error should be passed further instead IN> of being ignored. Acked-by: Peter Korsgaard -- Sorry about disclaimer - It's out of my control. Bye, Peter Korsgaard This message is subject to the following terms and conditions: MAIL DISCLAIMER<http://www.barco.com/en/maildisclaimer> -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] i2c-mux-gpio: use deferred probing with the device tree
>>>>> "IN" == Ionut Nicu writes: IN> If the i2c-parent bus driver is not loaded, returning IN> -EINVAL will force people to unload and then reload the IN> module again to get it working. IN> Signed-off-by: Ionut Nicu Acked-by: Peter Korsgaard -- Sorry about disclaimer - It's out of my control. Bye, Peter Korsgaard This message is subject to the following terms and conditions: MAIL DISCLAIMER<http://www.barco.com/en/maildisclaimer> -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] i2c-mux-gpio: use deferred probing with the device tree
>>>>> "IN" == Ionut Nicu writes: IN> If the i2c-parent bus driver is not loaded, returning IN> -EINVAL will force people to unload and then reload the IN> module again to get it working. IN> Also of_get_named_gpio could return -E_PROBE_DEFER or IN> another error code. This error should be passed further IN> instead of being ignored. Two different fixes, so should be 2 separate patches. Other that that, it looks good. Acked-by: Peter Korsgaard -- Sorry about disclaimer - It's out of my control. Bye, Peter Korsgaard This message is subject to the following terms and conditions: MAIL DISCLAIMER<http://www.barco.com/en/maildisclaimer> -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 1/3] i2c: mux: Add i2c-arb-gpio-challenge 'mux' driver
>>>>> "Wolfram" == Wolfram Sang writes: Hi, >> +- their-claim-gpios: The GPIOs that the other sides use the claim the bus. >> + Note that some implementations may only support a single other master. Wolfram> Stronger? "Currently, only one other master is supported"? Also there's a typo: s/use the claim/use to claim/ -- Bye, Peter Korsgaard -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" 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] i2c-mux-gpio: Check gpio_direction_output return value
>>>>> "JD" == Jean Delvare writes: JD> gpio_direction_output() may fail, check for that and deal with it JD> appropriately. Also log an error message if gpio_request() fails. JD> Signed-off-by: Jean Delvare JD> Cc: Peter Korsgaard JD> Cc: Wolfram Sang Acked-by: Peter Korsgaard -- Sorry about disclaimer - It's out of my control. Bye, Peter Korsgaard DISCLAIMER: Unless indicated otherwise, the information contained in this message is privileged and confidential, and is intended only for the use of the addressee(s) named above and others who have been specifically authorized to receive it. If you are not the intended recipient, you are hereby notified that any dissemination, distribution or copying of this message and/or attachments is strictly prohibited. The company accepts no liability for any damage caused by any virus transmitted by this email. Furthermore, the company does not warrant a proper and complete transmission of this information, nor does it accept liability for any delays. If you have received this message in error, please contact the sender and delete the message. Thank you. -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] i2c: busses: i2c-ocores: Fix PM-related warning
>>>>> "Maxin" == Maxin B John writes:
Maxin> On Tue, Feb 26, 2013 at 10:53 PM, wrote:
>> From: "Maxin B. John"
>>
>> Fixes this warning:
>> CC drivers/i2c/busses/i2c-ocores.o
>> drivers/i2c/busses/i2c-ocores.c:460:12: warning: 'ocores_i2c_suspend'
>> defined but not used [-Wunused-function]
>> drivers/i2c/busses/i2c-ocores.c:471:12: warning: 'ocores_i2c_resume'
>> defined but not used [-Wunused-function]
>>
>> Signed-off-by: Maxin B. John
>> ---
>> drivers/i2c/busses/i2c-ocores.c |9 +++--
>> 1 files changed, 3 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/i2c/busses/i2c-ocores.c
>> b/drivers/i2c/busses/i2c-ocores.c
>> index 0e1f824..1ecea5e 100644
>> --- a/drivers/i2c/busses/i2c-ocores.c
>> +++ b/drivers/i2c/busses/i2c-ocores.c
>> @@ -456,7 +456,7 @@ static int ocores_i2c_remove(struct platform_device
>> *pdev)
>> return 0;
>> }
>>
>> -#ifdef CONFIG_PM
>> +#ifdef CONFIG_PM_SLEEP
>> static int ocores_i2c_suspend(struct device *dev)
>> {
>> struct ocores_i2c *i2c = dev_get_drvdata(dev);
>> @@ -476,12 +476,9 @@ static int ocores_i2c_resume(struct device *dev)
>>
>> return 0;
>> }
>> +#endif
>>
>> static SIMPLE_DEV_PM_OPS(ocores_i2c_pm, ocores_i2c_suspend,
>> ocores_i2c_resume);
A comment explaining that the suspend/resume callbacks are unused in the
!PM_SLEEP mode would be good.
With this change we waste ~100 bytes on a dummy dev_pm_ops structure,
but OK.
>> -#define OCORES_I2C_PM (&ocores_i2c_pm)
>> -#else
>> -#define OCORES_I2C_PM NULL
>> -#endif
>>
>> static struct platform_driver ocores_i2c_driver = {
>> .probe = ocores_i2c_probe,
>> @@ -490,7 +487,7 @@ static struct platform_driver ocores_i2c_driver = {
>> .owner = THIS_MODULE,
>> .name = "ocores-i2c",
>> .of_match_table = ocores_i2c_match,
>> - .pm = OCORES_I2C_PM,
>> + .pm = &ocores_i2c_pm,
>> },
>> };
>>
>> --
>> 1.7.7
>>
Maxin> ping..
--
Bye, Peter Korsgaard
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] i2c-mux-gpio: Check gpio_direction_output return value
>>>>> "JD" == Jean Delvare writes:
JD> gpio_direction_output() may fail, check for that and deal with it
JD> appropriately. Also log an error message if gpio_request() fails.
JD> Signed-off-by: Jean Delvare
JD> Cc: Peter Korsgaard
JD> Cc: Wolfram Sang
JD> ---
JD> drivers/i2c/muxes/i2c-mux-gpio.c | 17 ++---
JD> 1 file changed, 14 insertions(+), 3 deletions(-)
JD> --- linux-3.7.orig/drivers/i2c/muxes/i2c-mux-gpio.c 2012-12-15
14:52:47.368991730 +0100
JD> +++ linux-3.7/drivers/i2c/muxes/i2c-mux-gpio.c 2013-03-03
14:27:21.003905620 +0100
JD> @@ -123,10 +123,21 @@ static int __devinit i2c_mux_gpio_probe(
JD> for (i = 0; i < pdata->n_gpios; i++) {
JD> ret = gpio_request(gpio_base + pdata->gpios[i], "i2c-mux-gpio");
JD> - if (ret)
JD> + if (ret) {
JD> + dev_err(&pdev->dev, "Failed to request GPIO %d\n",
JD> + pdata->gpios[i]);
JD> goto err_request_gpio;
JD> - gpio_direction_output(gpio_base + pdata->gpios[i],
JD> - initial_state & (1 << i));
JD> + }
JD> +
JD> + ret = gpio_direction_output(gpio_base + pdata->gpios[i],
JD> + initial_state & (1 << i));
JD> + if (ret) {
JD> + dev_err(&pdev->dev,
JD> + "Failed to set direction of GPIO %d to
output\n",
JD> + pdata->gpios[i]);
JD> + i++;
JD> + goto err_request_gpio;
The i++ is a bit nonobvious, so a comment would be good. Other than that
it looks fine.
Acked-by: Peter Korsgaard
--
Sorry about disclaimer - It's out of my control.
Bye, Peter Korsgaard
DISCLAIMER:
Unless indicated otherwise, the information contained in this message is
privileged and confidential, and is intended only for the use of the
addressee(s) named above and others who have been specifically authorized to
receive it. If you are not the intended recipient, you are hereby notified that
any dissemination, distribution or copying of this message and/or attachments
is strictly prohibited. The company accepts no liability for any damage caused
by any virus transmitted by this email. Furthermore, the company does not
warrant a proper and complete transmission of this information, nor does it
accept liability for any delays. If you have received this message in error,
please contact the sender and delete the message. Thank you.
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] i2c: ocores: Fix pointer to integer cast warning
>>>>> "Jayachandran" == Jayachandran C writes:
Jayachandran> After commit a000b8c1 [i2c: ocores: Add support for the
Jayachandran> GRLIB port of the controller and use function pointers
Jayachandran> for getreg and setreg function], compiling i2c-ocores.c
Jayachandran> for 64-bit gives the following warning:
Jayachandran> drivers/i2c/busses/i2c-ocores.c: In function
'ocores_i2c_of_probe':
Jayachandran> drivers/i2c/busses/i2c-ocores.c:334:15: warning: cast from
pointer to integer of different size
Jayachandran> Fix it by casting the pointer to long.
I've CC'ed Andreas who made the change.
Acked-by: Peter Korsgaard
Jayachandran> Signed-off-by: Jayachandran C
Jayachandran> ---
Jayachandran> drivers/i2c/busses/i2c-ocores.c |2 +-
Jayachandran> 1 file changed, 1 insertion(+), 1 deletion(-)
Jayachandran> diff --git a/drivers/i2c/busses/i2c-ocores.c
b/drivers/i2c/busses/i2c-ocores.c
Jayachandran> index a873d0a..6afa02d 100644
Jayachandran> --- a/drivers/i2c/busses/i2c-ocores.c
Jayachandran> +++ b/drivers/i2c/busses/i2c-ocores.c
Jayachandran> @@ -331,7 +331,7 @@ static int ocores_i2c_of_probe(struct
platform_device *pdev,
Jayachandran> &i2c->reg_io_width);
Jayachandran> match = of_match_node(ocores_i2c_match,
pdev->dev.of_node);
Jayachandran> -if (match && (int)match->data == TYPE_GRLIB) {
Jayachandran> +if (match && (long)match->data == TYPE_GRLIB) {
Jayachandran> dev_dbg(&pdev->dev, "GRLIB variant of
i2c-ocores\n");
i2c-> setreg = oc_setreg_grlib;
i2c-> getreg = oc_getreg_grlib;
Jayachandran> --
Jayachandran> 1.7.9.5
Jayachandran> --
Jayachandran> To unsubscribe from this list: send the line "unsubscribe
linux-i2c" in
Jayachandran> the body of a message to majord...@vger.kernel.org
Jayachandran> More majordomo info at
http://vger.kernel.org/majordomo-info.html
--
Bye, Peter Korsgaard
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 08/16] i2c: ocores: remove __dev* attributes
>>>>> "Bill" == Bill Pemberton writes: Bill> CONFIG_HOTPLUG is going away as an option. As result the __dev* Bill> markings will be going away. Bill> Remove use of __devinit, __devexit_p, __devinitdata, __devinitconst, Bill> and __devexit. Acked-by: Peter Korsgaard -- Bye, Peter Korsgaard -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 02/16] i2c: mux-gpio: remove __dev* attributes
>>>>> "BP" == Bill Pemberton writes: BP> CONFIG_HOTPLUG is going away as an option. As result the __dev* BP> markings will be going away. BP> Remove use of __devinit, __devexit_p, __devinitdata, __devinitconst, BP> and __devexit. BP> Signed-off-by: Bill Pemberton BP> Cc: Peter Korsgaard Acked-by: Peter Korsgaard -- Sorry about disclaimer - It's out of my control. Bye, Peter Korsgaard DISCLAIMER: Unless indicated otherwise, the information contained in this message is privileged and confidential, and is intended only for the use of the addressee(s) named above and others who have been specifically authorized to receive it. If you are not the intended recipient, you are hereby notified that any dissemination, distribution or copying of this message and/or attachments is strictly prohibited. The company accepts no liability for any damage caused by any virus transmitted by this email. Furthermore, the company does not warrant a proper and complete transmission of this information, nor does it accept liability for any delays. If you have received this message in error, please contact the sender and delete the message. Thank you. -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] i2c: i2c-ocores: Move grlib set/get functions into #ifdef CONFIG_OF block
>>>>> "Andreas" == Andreas Larsson writes: Andreas> This moves the grlib set and get functions into the #ifdef Andreas> CONFIG_OF block to avoid warnings of unimplemented functions Andreas> when compiling with -Wunused-function when CONFIG_OF is not Andreas> defined. Andreas> Signed-off-by: Andreas Larsson Acked-by: Peter Korsgaard -- Bye, Peter Korsgaard -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 2/2] i2c: i2c-ocores: Add support for the GRLIB port of the controller and use function pointers for getreg and setreg functions
>>>>> "Andreas" == Andreas Larsson writes: Andreas> The registers in the GRLIB port of the controller are 32-bit Andreas> and in big endian byte order. The PRELOW and PREHIGH registers Andreas> are merged into one register. The subsequent registers have Andreas> their offset decreased accordingly. Hence the register access Andreas> needs to be handled in a non-standard manner using custom Andreas> getreg and setreg functions. Andreas> Add setreg and getreg functions for different register widths Andreas> and let oc_setreg and oc_getreg use function pointers to call Andreas> the appropriate functions. Andreas> A type is added as the data of the of match table entries. A Andreas> new entry with a different compatible string is added to the Andreas> table. The type of that entry triggers usage of the custom Andreas> grlib functions by setting the setreg and getreg function Andreas> pointers. Andreas> Signed-off-by: Andreas Larsson Acked-by: Peter Korsgaard Thanks! -- Bye, Peter Korsgaard -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 2/2] i2c: i2c-ocores: Add support for the GRLIB port of the controller and use function pointers for getreg and setreg functions
>>>>> "Andreas" == Andreas Larsson writes:
Andreas> The registers in the GRLIB port of the controller are 32-bit
Andreas> and in big endian byte order. The PRELOW and PREHIGH registers
Andreas> are merged into one register. The subsequent registers have
Andreas> their offset decreased accordingly. Hence the register access
Andreas> needs to be handled in a non-standard manner using custom
Andreas> getreg and setreg functions.
Andreas> The single oc_getreg and one oc_setreg functions, that
Andreas> branches and call different ioread and iowrite functions, are
Andreas> replaced by specific functions that uses the appropriate
Andreas> ioread and iowrite functions and function pointers are
Andreas> initiated and used to call the approproate functions.
Andreas> A type is added as the data of the of match table entries. A
Andreas> new entry with a different compatible string is added to the
Andreas> table. The type of that entry triggers usage of the custom
Andreas> grlib functions by setting the new getreg and setreg function
Andreas> pointers to these functions.
Sorry to keep on requesting changes, but see below for a bit more:
Andreas> static void ocores_process(struct ocores_i2c *i2c)
Andreas> {
Andreas> struct i2c_msg *msg = i2c->msg;
Andreas> - u8 stat = oc_getreg(i2c, OCI2C_STATUS);
Andreas> + u8 stat = i2c->getreg(i2c, OCI2C_STATUS);
The patch would have been quite a bit smaller/easier to read if you had
kept oc_getreg / oc_setreg as wrappers.
Andreas> #ifdef CONFIG_OF
Andreas> static int ocores_i2c_of_probe(struct platform_device *pdev,
Andreas> struct ocores_i2c *i2c)
Andreas> {
Andreas> struct device_node *np = pdev->dev.of_node;
Andreas> + const struct of_device_id *match;
Andreas> u32 val;
Andreas> if (of_property_read_u32(np, "reg-shift", &i2c->reg_shift)) {
Andreas> @@ -257,6 +324,14 @@ static int ocores_i2c_of_probe(struct
platform_device *pdev,
Andreas> of_property_read_u32(pdev->dev.of_node, "reg-io-width",
Andreas> &i2c->reg_io_width);
Andreas> +
Andreas> + match = of_match_node(ocores_i2c_match, pdev->dev.of_node);
Andreas> + if (match && (int)match->data == TYPE_GRLIB) {
Andreas> + dev_dbg(&pdev->dev, "GRLIB variant of i2c-ocores\n");
Andreas> + i2c->setreg = oc_setreg_grlib;
Andreas> + i2c->getreg = oc_getreg_grlib;
Andreas> + }
Andreas> +
Andreas> return 0;
Andreas> }
Andreas> #else
Andreas> @@ -311,6 +386,23 @@ static int __devinit ocores_i2c_probe(struct
platform_device *pdev)
Andreas> if (i2c->reg_io_width == 0)
i2c-> reg_io_width = 1; /* Set to default value */
Andreas> + if (!i2c->getreg) {
Andreas> + if (i2c->reg_io_width == 4)
Andreas> + i2c->getreg = oc_getreg_32;
Andreas> + else if (i2c->reg_io_width == 2)
Andreas> + i2c->getreg = oc_getreg_16;
Andreas> + else
Andreas> + i2c->getreg = oc_getreg_8;
Andreas> + }
Andreas> + if (!i2c->setreg) {
Andreas> + if (i2c->reg_io_width == 4)
Andreas> + i2c->setreg = oc_setreg_32;
Andreas> + else if (i2c->reg_io_width == 2)
Andreas> + i2c->setreg = oc_setreg_16;
Andreas> + else
Andreas> + i2c->setreg = oc_setreg_8;
Andreas> + }
Andreas> +
These are always set together (could even move to a single ops
structure), so it would be shorter to assign them at the same time:
if (!i2c->setreg || !i2c->getreg) {
switch (i2c->reg_io_width) {
case 1:
i2c->setreg = oc_setreg_8;
i2c->getreg = oc_getreg_8;
break;
case 2:
i2c->setreg = oc_setreg_16;
...
default:
dev_err(&pdev->dev "Unsupported I/O width (%d)\n", i2c->reg_io_width);
return -EINVAL;
}
}
--
Bye, Peter Korsgaard
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 2/2] i2c: i2c-ocores: Add support for the GRLIB port of the controller and custom getreg and setreg functions
>>>>> "Andreas" == Andreas Larsson writes:
Hi,
Andreas> The registers in the GRLIB port of the controller are 32-bit
Andreas> and in big endian byte order. The PRELOW and PREHIGH registers
Andreas> are merged into one register. The subsequent registers have
Andreas> their offset decreased accordingly. Hence the register access
Andreas> needs to be handled in a non-standard manner using custom
Andreas> getreg and setreg functions.
Andreas> A type is added as the data of the of match table entries. A
Andreas> new entry with a different compatible string is added to the
Andreas> table. The type of that entry triggers usage of the grlib
Andreas> functions.
Andreas> Signed-off-by: Andreas Larsson
Andreas> ---
Andreas> Changes since v3:
Andreas> - Use a separate entry in the of match table for the grlib
Andreas> variant and trigger grlib function usage on type put in the
Andreas> data field of that table entry
Thanks. A few more comments:
Andreas> static inline void oc_setreg(struct ocores_i2c *i2c, int reg, u8
value)
Andreas> {
Andreas> - if (i2c->reg_io_width == 4)
Andreas> + if (i2c->setreg)
Andreas> + i2c->setreg(i2c, reg, value);
Andreas> + else if (i2c->reg_io_width == 4)
Andreas> iowrite32(value, i2c->base + (reg << i2c->reg_shift));
Andreas> else if (i2c->reg_io_width == 2)
Andreas> iowrite16(value, i2c->base + (reg << i2c->reg_shift));
It would have been nice to add oc_getreg_8/16/32 functions and always
use the function pointers - But ok, that can be done later.
Andreas> #ifdef CONFIG_OF
Andreas> +static int ocores_i2c_get_type(struct platform_device *pdev);
Andreas> +
Why not just move the implementation up here instead of the forward
declaration?
Andreas> +static int ocores_i2c_get_type(struct platform_device *pdev)
Andreas> +{
Andreas> + const struct of_device_id *match;
Andreas> +
Andreas> + match = of_match_node(ocores_i2c_match, pdev->dev.of_node);
Andreas> + if (match)
Andreas> + return (int)match->data;
Can this ever fail? If not, you might as well do the of_match_node
inline in the probe instead of this helper.
Other than that it looks good.
--
Bye, Peter Korsgaard
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" 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/2] i2c: i2c-ocores: Add support for the GRLIB port of the controller and custom getreg and setreg functions
>>>>> "Andreas" == Andreas Larsson writes: Hi, >> Adding a type define (TYPE_OCORES / TYPE_GRLIB) and a 2nd >> of_device_id entry with .data = TYPE_GRLIB, and then using that in >> the probe routine would be nicer. Have a look at i2c-at91.c for an >> example of a driver doing something like that. Andreas> Yes, that is a good idea. Do you think casting to and from Andreas> void * in the following solution is too ugly and rather have a Andreas> struct pointed to, or do you think that would be unnecessary? I find the casting OK. Thanks. -- Bye, Peter Korsgaard -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" 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/2] i2c: i2c-ocores: Add support for the GRLIB port of the controller and custom getreg and setreg functions
>>>>> "Andreas" == Andreas Larsson writes:
Hi,
Andreas> The registers in the GRLIB port of the controller are 32-bit
Andreas> and in big endian byte order. The PRELOW and PREHIGH registers
Andreas> are merged into one register. The subsequent registers have
Andreas> their offset decreased accordingly. Hence the register access
Andreas> needs to be handled in a non-standard manner using custom
Andreas> getreg and setreg functions.
Andreas> @@ -257,6 +300,14 @@ static int ocores_i2c_of_probe(struct
platform_device *pdev,
Andreas> of_property_read_u32(pdev->dev.of_node, "reg-io-width",
Andreas> &i2c->reg_io_width);
Andreas> +
Andreas> + if (of_device_is_compatible(pdev->dev.of_node,
Andreas> + "aeroflexgaisler,i2cmst")) {
Andreas> + dev_dbg(&pdev->dev, "GRLIB variant of i2c-ocores\n");
Andreas> + i2c->setreg = oc_setreg_grlib;
Andreas> + i2c->getreg = oc_getreg_grlib;
Andreas> + }
Andreas> +
Please also update the bindings documentation under
Documentation/devicetree/bindings/i2c.
With this core you need to add both aeroflexgaisler,i2cmst and
opencores,i2c-ocores to the compatible property, but the grlib variant
is NOT compatible with i2c-ocores, so that's not really nice.
Adding a type define (TYPE_OCORES / TYPE_GRLIB) and a 2nd of_device_id
entry with .data = TYPE_GRLIB, and then using that in the probe routine
would be nicer. Have a look at i2c-at91.c for an example of a driver
doing something like that.
--
Bye, Peter Korsgaard
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] i2c: mux: Add dt support to i2c-mux-gpio driver
>>>>> "MR" == Maxime Ripard writes:
Hi,
MR> Allow the i2c-mux-gpio to be used by a device tree enabled device. The
MR> bindings are inspired by the one found in the i2c-mux-pinctrl driver.
MR> Signed-off-by: Maxime Ripard
MR> Reviewed-by: Stephen Warren
MR> ---
MR> .../devicetree/bindings/i2c/i2c-mux-gpio.txt | 81 +++
MR> drivers/i2c/muxes/i2c-mux-gpio.c | 146
+++-
MR> 2 files changed, 196 insertions(+), 31 deletions(-)
MR> create mode 100644 Documentation/devicetree/bindings/i2c/i2c-mux-gpio.txt
MR> diff --git a/Documentation/devicetree/bindings/i2c/i2c-mux-gpio.txt
b/Documentation/devicetree/bindings/i2c/i2c-mux-gpio.txt
MR> new file mode 100644
MR> index 000..d61726f
MR> --- /dev/null
MR> +++ b/Documentation/devicetree/bindings/i2c/i2c-mux-gpio.txt
MR> @@ -0,0 +1,81 @@
MR> +GPIO-based I2C Bus Mux
MR> +
MR> +This binding describes an I2C bus multiplexer that uses GPIOs to
MR> +route the I2C signals.
MR> +
MR> + +-+ +-+
MR> + | dev | | dev |
MR> ++++-+ +-+
MR> +| SoC| ||
MR> +|| /++
MR> +| +--+ | +--+child bus A, on GPIO value set to 0
MR> +| | I2C |-|--| Mux |
MR> +| +--+ | +--+---+child bus B, on GPIO value set to 1
MR> +|| |\--+++
MR> +| +--+ | | |||
MR> +| | GPIO |-|-++-+ +-+ +-+
MR> +| +--+ | | dev | | dev | | dev |
MR> +++ +-+ +-+ +-+
MR> +
MR> +Required properties:
MR> +- compatible: i2c-mux-gpio
MR> +- i2c-parent: The phandle of the I2C bus that this multiplexer's
master-side
MR> + port is connected to.
MR> +- mux-gpios: list of gpios used to control the muxer
MR> +* Standard I2C mux properties. See mux.txt in this directory.
MR> +* I2C child bus nodes. See mux.txt in this directory.
MR> +
MR> +Optional properties:
MR> +- idle-state: value to set to the muxer when idle. When no value is
s/to set to the muxer/to set the muxer to/
MR> +++ b/drivers/i2c/muxes/i2c-mux-gpio.c
MR> @@ -16,6 +16,8 @@
MR> #include
MR> #include
MR> #include
MR> +#include
MR> +#include
MR> struct gpiomux {
MR> struct i2c_adapter *parent;
MR> @@ -57,29 +59,111 @@ static int __devinit match_gpio_chip_by_label(struct
gpio_chip *chip,
MR> return !strcmp(chip->label, data);
MR> }
MR> +#ifdef CONFIG_OF
MR> +static int __devinit i2c_mux_gpio_probe_dt(struct gpiomux *mux,
MR> + struct platform_device *pdev)
MR> +{
MR> + struct device_node *np = pdev->dev.of_node;
MR> + struct device_node *adapter_np, *child;
MR> + struct i2c_adapter *adapter;
MR> + unsigned *values, *gpios;
MR> + int i = 0;
MR> +
MR> + if (!np)
MR> + return 0;
This should be -ENODEV, otherwise we end up using a zeroed out struct
i2c_mux_gpio_platform_data in case i2c_mux_gpio is used with the
platform bus but the platform forgets to pass the pdata.
With those two minor fixes:
Acked-by: Peter Korsgaard
--
Sorry about disclaimer - It's out of my control.
Bye, Peter Korsgaard
DISCLAIMER:
Unless indicated otherwise, the information contained in this message is
privileged and confidential, and is intended only for the use of the
addressee(s) named above and others who have been specifically authorized to
receive it. If you are not the intended recipient, you are hereby notified that
any dissemination, distribution or copying of this message and/or attachments
is strictly prohibited. The company accepts no liability for any damage caused
by any virus transmitted by this email. Furthermore, the company does not
warrant a proper and complete transmission of this information, nor does it
accept liability for any delays. If you have received this message in error,
please contact the sender and delete the message. Thank you.
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] i2c: i2c-ocores: Add support for the GRLIB port of the controller and custom getreg and setreg functions
>>>>> "Andreas" == Andreas Larsson writes: >> Are all platforms using i2c-ocores guaranteed to provide ioread32be / >> iowrite32be or should we stick an #ifdef CONFIG_SPARC around it? Andreas> As far as I can see, after digging around, the only platforms that Andreas> have ioread/write32, but not ioread/write32be are frv and mn10300. Do Andreas> you know if those platforms are using i2c-ocores? Not to my knowledge, no. In that case: Acked-by: Peter Korsgaard -- Bye, Peter Korsgaard -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] i2c: mux: Add dt support to i2c-mux-gpio driver
>>>>> "MR" == Maxime Ripard writes: Hi, MR> +* Standard I2C mux properties. See mux.txt in this directory. MR> +* I2C child bus nodes. See mux.txt in this directory. MR> + MR> +Optional properties: MR> +- idle-state: value to set to the muxer when idle. When no value is >> >> How about 'bitmask defining mux state when idle' instead? MR> Since the array documented as using the bitmasks in the platform_data MR> and described as an array of bitmasks is called "values", and the file MR> mux.txt talks about "numbers" to put into reg, won't this be confusing MR> to have three names for the exact same thing? Yeah, the mess is less than perfect. To my defense I did use bitmask in the platform data documentation: @values: Array of bitmasks of GPIO settings (low/high) for each position @idle: Bitmask to write to MUX when idle or GPIO_I2CMUX_NO_IDLE if not used But ok, I don't feel strongly about it. >> Why this change? I don't see why it is needed and the patch would be a >> lot easier to review without all the s/.data/->data/ noise. MR> Ah yes, since mux is already allocated using kcalloc, we don't need to MR> do it for data as well. I will remove it. Ok, great. -- Sorry about disclaimer - It's out of my control. Bye, Peter Korsgaard DISCLAIMER: Unless indicated otherwise, the information contained in this message is privileged and confidential, and is intended only for the use of the addressee(s) named above and others who have been specifically authorized to receive it. If you are not the intended recipient, you are hereby notified that any dissemination, distribution or copying of this message and/or attachments is strictly prohibited. The company accepts no liability for any damage caused by any virus transmitted by this email. Furthermore, the company does not warrant a proper and complete transmission of this information, nor does it accept liability for any delays. If you have received this message in error, please contact the sender and delete the message. Thank you. -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] i2c: i2c-ocores: Add support for the GRLIB port of the controller and custom getreg and setreg functions
>>>>> "Andreas" == Andreas Larsson writes:
Andreas> The registers in the GRLIB port of the controller are 32-bit
Andreas> and in big endian byte order. The PRELOW and PREHIGH registers
Andreas> are merged into one register. The subsequent registers have
Andreas> their offset decreased accordingly. Hence the register access
Andreas> needs to be handled in a non-standard manner using custom
Andreas> getreg and
Andreas> setreg functions.
Andreas> Signed-off-by: Andreas Larsson
Andreas> ---
Andreas> drivers/i2c/busses/i2c-ocores.c | 57
+-
Andreas> 1 files changed, 55 insertions(+), 2 deletions(-)
Andreas> diff --git a/drivers/i2c/busses/i2c-ocores.c
b/drivers/i2c/busses/i2c-ocores.c
Andreas> index 1eb8a65..e3df62f 100644
Andreas> --- a/drivers/i2c/busses/i2c-ocores.c
Andreas> +++ b/drivers/i2c/busses/i2c-ocores.c
Andreas> @@ -4,6 +4,9 @@
Andreas> *
Andreas> * Peter Korsgaard
Andreas> *
Andreas> + * Support for the GRLIB port of the controller by
Andreas> + * Andreas Larsson
Andreas> + *
Andreas> * This file is licensed under the terms of the GNU General Public
License
Andreas> * version 2. This program is licensed "as is" without any warranty
of any
Andreas> * kind, whether express or implied.
Andreas> @@ -38,6 +41,8 @@ struct ocores_i2c {
Andreas> int nmsgs;
Andreas> int state; /* see STATE_ */
Andreas> int clock_khz;
Andreas> + void (*setreg)(struct ocores_i2c *i2c, int reg, u8 value);
Andreas> + u8 (*getreg)(struct ocores_i2c *i2c, int reg);
Andreas> };
Andreas> /* registers */
Andreas> @@ -73,7 +78,9 @@ struct ocores_i2c {
Andreas> static inline void oc_setreg(struct ocores_i2c *i2c, int reg, u8
value)
Andreas> {
Andreas> - if (i2c->reg_io_width == 4)
Andreas> + if (i2c->setreg)
Andreas> + i2c->setreg(i2c, reg, value);
Andreas> + else if (i2c->reg_io_width == 4)
Andreas> iowrite32(value, i2c->base + (reg << i2c->reg_shift));
Andreas> else if (i2c->reg_io_width == 2)
Andreas> iowrite16(value, i2c->base + (reg << i2c->reg_shift));
Andreas> @@ -83,7 +90,9 @@ static inline void oc_setreg(struct ocores_i2c
*i2c, int reg, u8 value)
Andreas> static inline u8 oc_getreg(struct ocores_i2c *i2c, int reg)
Andreas> {
Andreas> - if (i2c->reg_io_width == 4)
Andreas> + if (i2c->getreg)
Andreas> + return i2c->getreg(i2c, reg);
Andreas> + else if (i2c->reg_io_width == 4)
Andreas> return ioread32(i2c->base + (reg << i2c->reg_shift));
Andreas> else if (i2c->reg_io_width == 2)
Andreas> return ioread16(i2c->base + (reg << i2c->reg_shift));
Andreas> @@ -91,6 +100,40 @@ static inline u8 oc_getreg(struct ocores_i2c
*i2c, int reg)
Andreas> return ioread8(i2c->base + (reg << i2c->reg_shift));
Andreas> }
Andreas> +/* Read and write functions for the GRLIB port of the controller.
Registers are
Andreas> + * 32-bit big endian and the PRELOW and PREHIGH registers are merged
into one
Andreas> + * register. The subsequent registers has their offset decreased
accordingly. */
Andreas> +static u8 oc_getreg_grlib(struct ocores_i2c *i2c, int reg)
Andreas> +{
Andreas> + u32 rd;
Andreas> + int rreg = reg;
Andreas> + if (reg != OCI2C_PRELOW)
Andreas> + rreg--;
Andreas> + rd = ioread32be(i2c->base + (rreg << i2c->reg_shift));
Andreas> + if (reg == OCI2C_PREHIGH)
Andreas> + return (u8)rd >> 8;
Andreas> + else
Andreas> + return (u8)rd;
Andreas> +}
Andreas> +
Andreas> +static void oc_setreg_grlib(struct ocores_i2c *i2c, int reg, u8
value)
Andreas> +{
Andreas> + u32 curr, wr;
Andreas> + int rreg = reg;
Andreas> + if (reg != OCI2C_PRELOW)
Andreas> + rreg--;
Andreas> + if (reg == OCI2C_PRELOW || reg == OCI2C_PREHIGH) {
Andreas> + curr = ioread32be(i2c->base + (rreg << i2c->reg_shift));
Andreas> + if (reg == OCI2C_PRELOW)
Andreas> + wr = (curr & 0xff00) | value;
Andreas> + else
Andreas> + wr = (((u32)value) << 8) | (curr & 0xff);
Andreas> + } else {
Andreas> + wr = value;
Andreas> + }
Andreas> + iowrite32be(wr, i2c->base + (rreg << i2c->reg_shift));
Are all platforms using i2c-ocores guaranteed to provide ioread32be /
iowrite32be or should we stick an #ifdef CONFIG_SPARC around it?
--
Bye, Peter Korsgaard
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] i2c: i2c-ocores: Add irq support for sparc
>>>>> "Andreas" == Andreas Larsson writes:
Andreas> There are no platform resources of type IORESOURCE_IRQ on
Andreas> sparc, so the irq number is acquired in a different manner for
Andreas> sparc. The general case uses platform_get_irq, that internally
Andreas> still uses platform_get_resource.
I have no idea why sparc is being odd in this regard, but assuming this
is how it's done, I'm fine with this change.
A quick grep doesn't find any other drivers doing this though:
git grep -l archdata.irqs drivers | xargs grep platform_get_irq
Acked-by: Peter Korsgaard
Andreas> Signed-off-by: Andreas Larsson
Andreas> ---
Andreas> drivers/i2c/busses/i2c-ocores.c | 13 +
Andreas> 1 files changed, 9 insertions(+), 4 deletions(-)
Andreas> diff --git a/drivers/i2c/busses/i2c-ocores.c
b/drivers/i2c/busses/i2c-ocores.c
Andreas> index bffd550..1eb8a65 100644
Andreas> --- a/drivers/i2c/busses/i2c-ocores.c
Andreas> +++ b/drivers/i2c/busses/i2c-ocores.c
Andreas> @@ -267,16 +267,21 @@ static int __devinit ocores_i2c_probe(struct
platform_device *pdev)
Andreas> {
Andreas> struct ocores_i2c *i2c;
Andreas> struct ocores_i2c_platform_data *pdata;
Andreas> - struct resource *res, *res2;
Andreas> + struct resource *res;
Andreas> int ret;
Andreas> int i;
Andreas> + int irq;
Andreas> res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
Andreas> if (!res)
Andreas> return -ENODEV;
Andreas> - res2 = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
Andreas> - if (!res2)
Andreas> +#ifdef CONFIG_SPARC
Andreas> + irq = pdev->archdata.irqs[0];
Andreas> +#else
Andreas> + irq = platform_get_irq(pdev, 0);
Andreas> +#endif
Andreas> + if (irq < 0)
Andreas> return -ENODEV;
Andreas> i2c = devm_kzalloc(&pdev->dev, sizeof(*i2c), GFP_KERNEL);
Andreas> @@ -313,7 +318,7 @@ static int __devinit ocores_i2c_probe(struct
platform_device *pdev)
Andreas> ocores_init(i2c);
Andreas> init_waitqueue_head(&i2c->wait);
Andreas> - ret = devm_request_irq(&pdev->dev, res2->start, ocores_isr, 0,
Andreas> + ret = devm_request_irq(&pdev->dev, irq, ocores_isr, 0,
pdev-> name, i2c);
Andreas> if (ret) {
Andreas> dev_err(&pdev->dev, "Cannot claim IRQ\n");
Andreas> --
Andreas> 1.7.0.4
Andreas> --
Andreas> To unsubscribe from this list: send the line "unsubscribe linux-i2c"
in
Andreas> the body of a message to majord...@vger.kernel.org
Andreas> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Bye, Peter Korsgaard
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] i2c: mux: Add dt support to i2c-mux-gpio driver
t;classes ? mux->data->classes[i]
: 0;
mux-> adap[i] = i2c_add_mux_adapter(parent, &pdev->dev, mux, nr,
MR>i, class,
MR> @@ -144,19 +230,17 @@ static int __devinit i2c_mux_gpio_probe(struct
platform_device *pdev)
MR> }
MR> dev_info(&pdev->dev, "%d port mux on %s adapter\n",
MR> -pdata->n_values, parent->name);
MR> -
MR> - platform_set_drvdata(pdev, mux);
MR> + mux->data->n_values, parent->name);
MR> return 0;
MR> add_adapter_failed:
MR> for (; i > 0; i--)
MR> i2c_del_mux_adapter(mux->adap[i - 1]);
MR> - i = pdata->n_gpios;
MR> + i = mux->data->n_gpios;
MR> err_request_gpio:
MR> for (; i > 0; i--)
MR> - gpio_free(gpio_base + pdata->gpios[i - 1]);
MR> + gpio_free(gpio_base + mux->data->gpios[i - 1]);
MR> alloc_failed:
MR> i2c_put_adapter(parent);
MR> @@ -168,11 +252,11 @@ static int __devexit i2c_mux_gpio_remove(struct
platform_device *pdev)
MR> struct gpiomux *mux = platform_get_drvdata(pdev);
MR> int i;
MR> - for (i = 0; i < mux->data.n_values; i++)
MR> + for (i = 0; i < mux->data->n_values; i++)
MR> i2c_del_mux_adapter(mux->adap[i]);
MR> - for (i = 0; i < mux->data.n_gpios; i++)
MR> - gpio_free(mux->gpio_base + mux->data.gpios[i]);
MR> + for (i = 0; i < mux->data->n_gpios; i++)
MR> + gpio_free(mux->gpio_base + mux->data->gpios[i]);
MR> platform_set_drvdata(pdev, NULL);
MR> i2c_put_adapter(mux->parent);
MR> @@ -180,12 +264,19 @@ static int __devexit i2c_mux_gpio_remove(struct
platform_device *pdev)
MR> return 0;
MR> }
MR> +static const struct of_device_id i2c_mux_gpio_of_match[] __devinitconst = {
MR> + { .compatible = "i2c-mux-gpio", },
MR> + {},
MR> +};
MR> +MODULE_DEVICE_TABLE(of, i2c_mux_gpio_of_match);
MR> +
MR> static struct platform_driver i2c_mux_gpio_driver = {
MR> .probe = i2c_mux_gpio_probe,
MR> .remove = __devexit_p(i2c_mux_gpio_remove),
MR> .driver = {
MR> .owner = THIS_MODULE,
MR> .name = "i2c-mux-gpio",
MR> + .of_match_table = of_match_ptr(i2c_mux_gpio_of_match),
MR> },
MR> };
MR> --
MR> 1.7.9.5
--
Sorry about disclaimer - It's out of my control.
Bye, Peter Korsgaard
DISCLAIMER:
Unless indicated otherwise, the information contained in this message is
privileged and confidential, and is intended only for the use of the
addressee(s) named above and others who have been specifically authorized to
receive it. If you are not the intended recipient, you are hereby notified that
any dissemination, distribution or copying of this message and/or attachments
is strictly prohibited. The company accepts no liability for any damage caused
by any virus transmitted by this email. Furthermore, the company does not
warrant a proper and complete transmission of this information, nor does it
accept liability for any delays. If you have received this message in error,
please contact the sender and delete the message. Thank you.
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] i2c: busses: i2c-ocores: switch to devm_request_and_ioremap
>>>>> "Maxin" == Maxin B John writes: Maxin> Hi Peter, Maxin> This drops a few lines of code and allows common APIs to handle Maxin> those for us. Maxin> Signed-off-by: Maxin B. John >> Acked-by: Peter Korsgaard Maxin> Is there any update on this ? I'm expecting Wolfram to pick it up. -- Bye, Peter Korsgaard -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] i2c: busses: i2c-ocores: switch to devm_request_and_ioremap
>>>>> "Maxin" == Maxin B John writes: Maxin> This drops a few lines of code and allows common APIs to handle Maxin> those for us. Maxin> Signed-off-by: Maxin B. John Acked-by: Peter Korsgaard -- Bye, Peter Korsgaard -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] i2c: busses: i2c-ocores: Fix documentation filename
>>>>> "Maxin" == Maxin B John writes: Maxin> Fixes the wrong filename. Maxin> Signed-off-by: Maxin B. John Acked-by: Peter Korsgaard -- Bye, Peter Korsgaard -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] i2c: i2c-mux-gpio: Use devm_kzalloc instead of kzalloc
>>>>> "Maxime" == Maxime Ripard writes: Maxime> Use the devm_kzalloc managed function to stripdown the error and remove Maxime> code. Maxime> Signed-off-by: Maxime Ripard Besides the comment of Jean - Acked-by: Peter Korsgaard -- Bye, Peter Korsgaard -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/4] i2c: i2c-ocores: Use reg-shift property
>>>>> "J" == Jayachandran C writes:
J> From: Ganesan Ramalingam
J> Deprecate 'regstep' property and use the standard 'reg-shift' property
J> for register offset shifts. 'regstep' will still be supported as an
J> optional property, but will give a warning when used.
..
J> struct ocores_i2c_platform_data {
J> - u32 regstep; /* distance between registers */
J> - u32 clock_khz; /* input clock in kHz */
J> - u8 num_devices; /* number of devices in the devices list */
J> + u32 reg_shift; /* register offset shift value */
J> + u32 clock_khz; /* input clock in kHz */
J> + u8 num_devices; /* number of devices in the devices list */
J> struct i2c_board_info const *devices; /* devices connected to the bus */
J> };
Why not just keep this change to the dt bindings, instead of risking
breaking stuff for platform drivers as well? There's no conceptual
reason why reg_shift is any better than regstep.
--
Bye, Peter Korsgaard
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] i2c-ocore: register OF i2c devices
>>>>> "Wolfram" == Wolfram Sang writes: Wolfram> On Tue, May 08, 2012 at 06:55:28PM +0530, Jayachandran C wrote: >> From: Ganesan Ramalingam >> >> Call of_i2c_register_devices() in probe function to register i2c devices >> specified in the device tree or OF. >> >> Signed-off-by: Ganesan Ramalingam >> Signed-off-by: Jayachandran C Wolfram> Applied to next with a minor fixup. Wolfram> More important: If you are happily using this driver without issues, Wolfram> please consider removing EXPERIMENTAL for this driver from Kconfig. And also please consider CC'ing the maintainer on patches (E.G. me) - I didn't notice this before now. -- Bye, Peter Korsgaard -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] i2c: Rename last mux driver to standard pattern
>>>>> "JD" == Jean Delvare writes: JD> Update the MAINTAINERS entry and all other references accordingly. JD> Based on an original patch by Wolfram Sang. JD> Signed-off-by: Jean Delvare JD> Acked-by: Peter Korsgaard JD> Cc: Wolfram Sang JD> --- JD> Peter, I assumed your ack still applied, please yell if not. It does, thanks. JD> Changes since v1: JD> * Renamed header file and documentation file too. JD> * Updated MAINTAINERS. JD> * Updated references. JD> Documentation/i2c/muxes/gpio-i2cmux | 65 JD> Documentation/i2c/muxes/i2c-mux-gpio | 65 JD> MAINTAINERS |6 - JD> drivers/i2c/muxes/Kconfig|2 JD> drivers/i2c/muxes/Makefile |2 JD> drivers/i2c/muxes/gpio-i2cmux.c | 173 -- JD> drivers/i2c/muxes/i2c-mux-gpio.c | 173 ++ JD> include/linux/gpio-i2cmux.h | 38 --- JD> include/linux/i2c-mux-gpio.h | 38 +++ git diff -M patches are a lot easier to read for this kind of change. -- Bye, Peter Korsgaard DISCLAIMER: Unless indicated otherwise, the information contained in this message is privileged and confidential, and is intended only for the use of the addressee(s) named above and others who have been specifically authorized to receive it. If you are not the intended recipient, you are hereby notified that any dissemination, distribution or copying of this message and/or attachments is strictly prohibited. The company accepts no liability for any damage caused by any virus transmitted by this email. Furthermore, the company does not warrant a proper and complete transmission of this information, nor does it accept liability for any delays. If you have received this message in error, please contact the sender and delete the message. Thank you. -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] i2c-mux-gpio: Rename platform data structure
>>>>> "JD" == Jean Delvare writes: JD> Align the name of i2c-mux-gpio's data structure on the new driver name. JD> Also change one define and adjust function names, even if they aren't JD> part of the public interface, for consistency. JD> Signed-off-by: Jean Delvare JD> Cc: Peter Korsgaard JD> Cc: Wolfram Sang Acked-by: Peter Korsgaard -- Bye, Peter Korsgaard DISCLAIMER: Unless indicated otherwise, the information contained in this message is privileged and confidential, and is intended only for the use of the addressee(s) named above and others who have been specifically authorized to receive it. If you are not the intended recipient, you are hereby notified that any dissemination, distribution or copying of this message and/or attachments is strictly prohibited. The company accepts no liability for any damage caused by any virus transmitted by this email. Furthermore, the company does not warrant a proper and complete transmission of this information, nor does it accept liability for any delays. If you have received this message in error, please contact the sender and delete the message. Thank you. -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] i2c: muxes are not EXPERIMENTAL anymore
>>>>> "Wolfram" == Wolfram Sang writes: Wolfram> We got multiple patches to add mux support to device tree, so Wolfram> people are using it happily already and build up on it. I also Wolfram> used it in a project without encountering problems. 20 months Wolfram> of EXPERIMENTAL should do for this. Agreed. Acked-by: Peter Korsgaard -- Bye, Peter Korsgaard -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] i2c: reorganize muxes to a standard pattern
>>>>> "Wolfram" == Wolfram Sang writes: >> > > Wolfram, Peter, any progress here? I think Wolfram was supposed to send >> > > an updated patch but I did not receive anything. >> > >> > -EBUSY :( >> > >> > It *is* on my todo-list, though... >> >> Any news on this? Wolfram> Well, it is still on my list :/ (I wouldn't mind if someone is Wolfram> faster than me) I'll be away for the next 2 weeks, but will do it afterwards if nobody beats me to it (and I don't forget). -- Bye, Peter Korsgaard -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] i2c: convert gpio-i2cmux to use module_platform_driver()
>>>>> "AL" == Axel Lin writes: AL> This patch converts gpio-i2cmux to use the module_platform_driver() macro which AL> makes the code smaller and a bit simpler. AL> Signed-off-by: Axel Lin Acked-by: Peter Korsgaard -- Sorry about disclaimer - It's out of my control. Bye, Peter Korsgaard DISCLAIMER: Unless indicated otherwise, the information contained in this message is privileged and confidential, and is intended only for the use of the addressee(s) named above and others who have been specifically authorized to receive it. If you are not the intended recipient, you are hereby notified that any dissemination, distribution or copying of this message and/or attachments is strictly prohibited. The company accepts no liability for any damage caused by any virus transmitted by this email. Furthermore, the company does not warrant a proper and complete transmission of this information, nor does it accept liability for any delays. If you have received this message in error, please contact the sender and delete the message. Thank you. -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] i2c: reorganize muxes to a standard pattern
>>>>> "Wolfram" == Wolfram Sang writes: >> > http://thread.gmane.org/gmane.linux.drivers.i2c/7171/focus=7244 >> >> Ah, yes, that's the discussion I was looking for, thanks for digging it >> out. My point wasn't totally wrong back then, but Wolfram's is simply >> better, I admit. Wolfram> I agree :) Okay, so I'll update the documentation as well. What about Wolfram> include/linux/gpio-i2cmux.h and its users? I'd like consistency, but Wolfram> renaming header files is not too nice... Indeed. If we were to rename it we should also rename struct gpio_i2cmux_platform_data. I don't feel strongly about it - It will break for existing users, but there's probably not too many of those. Your call. -- Bye, Peter Korsgaard -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] i2c: reorganize muxes to a standard pattern
>>>>> "Jean" == Jean Delvare writes:
Hi,
>> rename drivers/i2c/muxes/{gpio-i2cmux.c => i2c-mux-gpio.c} (100%)
>> rename drivers/i2c/muxes/{pca9541.c => i2c-mux-pca9541.c} (100%)
>> rename drivers/i2c/muxes/{pca954x.c => i2c-mux-pca954x.c} (100%)
Jean> You forgot to rename Documentation/i2c/muxes/gpio-i2cmux.
Jean> I thought the naming had been discussed before, but I can't find the
Jean> references, and to be honest the new names please me, so I have no
Jean> reason to decline your proposal. I would have appreciated an ack from
Jean> the driver authors (Cc'd) though. I'll apply your patch as soon as I
Jean> receive an updated version, unless I get an objection before then.
Are you referring to this?
http://thread.gmane.org/gmane.linux.drivers.i2c/7171/focus=7244
I originally called it i2c-gpiomux.c, and renamed it to gpio-i2cmux.c on
request of you.
I don't mind the new name though, so:
Acked-by: Peter Korsgaard
--
Bye, Peter Korsgaard
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] i2c: convert drivers/i2c/* to use module_platform_driver()
>>>>> "Axel" == Axel Lin writes: Axel> This patch converts the drivers in drivers/i2c/* to use the Axel> module_platform_driver() macro which makes the code smaller and a bit Axel> simpler. Axel> Cc: Jean Delvare Axel> Cc: Ben Dooks Axel> Cc: Jochen Friedrich Axel> Cc: Peter Korsgaard Axel> Cc: Wolfram Sang Axel> Cc: Guan Xuetao Axel> Cc: Manuel Lauss Axel> Cc: Barry Song <21cn...@gmail.com> Axel> Cc: Linus Walleij Axel> Cc: Yong Zhang Axel> Cc: Lucas De Marchi Axel> Cc: Grant Likely Axel> Cc: Samuel Ortiz Axel> Signed-off-by: Axel Lin Looks good for i2c-ocores / gpio-i2cmux: Acked-by: Peter Korsgaard -- Bye, Peter Korsgaard -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: i2c-ocores timeout bug
>>>>> "Jean" == Jean Delvare writes:
Hi,
>> Basically the i2c-ocores xfer function, kicks off an interrupt driven
>> array of i2c_msg transfers and waits 1second for it to complete and
>> just
>> returns -ETIMEDOUT if it times out. The interrupt driven transfer is
>> *not* stopped on a time out and continues referencing the i2c_msg
>> structure. In my
>> case an i2c read the interrupt handler keeps adding bytes to the
>> structure. Unfortunately after the time out the structure was released
>> and it's length field was changed to a large number and the interrupt
>> driver kept happily writing bytes way past the end of the original
>> buffer until the kernel crashed or locked up!
>>
>> Below is a fix that works for me in 2.6.32 which removes the i2c_msg
>> buffer from the interrupt handler and changes the handler to check for
>> it's removal.
There's been some changes to the ocores drivers since then (mainly
device tree support) - It would be good with a patch that can be applied
to head.
>>
>> diff --git a/drivers/i2c/busses/i2c-ocores.c
>> b/drivers/i2c/busses/i2c-ocores.c
>> index 8dee246..21e57a0 100644
>> --- a/drivers/i2c/busses/i2c-ocores.c
>> +++ b/drivers/i2c/busses/i2c-ocores.c
>> @@ -137,6 +137,12 @@ static void ocores_process(struct ocores_i2c *i2c)
>> wake_up(&i2c->wait);
>> return;
>> }
>> + if (msg == 0) {
>> + /* Caller has with drawn the request, buffer no longer available */
>> + i2c->state = STATE_ERROR;
>> + oc_setreg(i2c, OCI2C_CMD, OCI2C_CMD_STOP);
>> + return;
>> + }
I would prefer to send a stop here. You don't know in what context this
will get called (spurious inteerupts?). I also don't like adding extra
meaning i2c->msg when we have a perfectly fine state variable.
How about you instead set state to STATE_ERROR in ocores_xfer on
timeouts, and then directly send the stop condition (so not in the
isr). Otherwise you don't have any guarantee that the next transfer
isn't setup by the time the interrupt fires.
>>
>> /* error? */
>> if (stat & OCI2C_STAT_ARBLOST) {
>> @@ -223,8 +229,13 @@ static int ocores_xfer(struct i2c_adapter *adap,
>> struct i2c_msg *msgs, int num)
>> if (wait_event_timeout(i2c->wait, (i2c->state == STATE_ERROR) ||
>> (i2c->state == STATE_DONE), HZ))
>> return (i2c->state == STATE_DONE) ? num : -EIO;
>> - else
>> + else {
>> + printk(KERN_NOTICE
WARNING seems more suitable here.
>> + "ocores_xfer() i2c transfer to %d-%#x timed out\n",
>> + i2c_adapter_id(adap), msgs->addr);
>> + i2c->msg = 0; /* remove the caller's request which
>> will be re-used */
>> return -ETIMEDOUT;
>> + }
>> }
--
Bye, Peter Korsgaard
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: i2c-ocores timeout bug
>>>>> "Andrew" == Andrew Worsley writes: Andrew> So far no response on this bug I reported about 2 weeks ago that Andrew> includes a proposed fix. Andrew> Could some one consider adding in the fix into the relevant tree? Sorry, things got somewhat delayed because: - You didn't email me (the maintainer) - I've been caught up with real life stuff (my daughter has been ill) I'll take a look at your patch in detail later today. Andrew> On 15 June 2011 08:33, Andrew Worsley wrote: >> Hi, I have hit upon a bug in this driver in the 2.6.32 which caused >> memory corrupt and crash in my kernel. It appears to be still present >> in 3.0-rc3 Andrew> http://thread.gmane.org/gmane.linux.drivers.i2c/8543 Andrew> I have since identified the cause of the problem was the wrong clock Andrew> frequency for the FPGA. I mis-understood the clock frequency to Andrew> be the i2c bus frequency - but it is actually an FPGA clock frequency. Andrew> Perhaps others will make the same mistake as well. Ahh yes, I did find your timeout issues odd. It is indeed the input clock to the ocores IP block, like it's documented in the i2c-ocores vhdl spec. In the years i2c-ocores has been in the tree, this is the first time I ever heard about anyone making that mistake though. The documentation of the platform data also says: u32 clock_khz; /* input clock in kHz */ Andrew> In fact as it stands there is no module parameter to change your i2c Andrew> bus frequency - other than by fiddling your FPGA clock frequency. Andrew> Better would be a separate parameters for each with a clearer names. Yes, currently the i2c speed is always the standard 100KHz. I'm not opposed to a patch adding a i2c_clock_khz member to ocores_i2c_platform_data as long as it doesn't break existing boards (E.G. 0 should get handled as 100Khz). -- Bye, Peter Korsgaard -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Warning with gpio-i2cmux
>>>>> "JD" == Jean Delvare writes: Hi, JD> I presume you are using gpio-i2cmux on some of your systems? I tried to JD> use it and I get the following warning when I rmmod the underlying I2C JD> controller driver: Pulling the rug under the driver is not something I normally do, but it should work. JD> WARNING: at drivers/base/core.c:143 device_release+0x82/0x90() JD> Hardware name: System Product Name JD> Device 'gpio-i2cmux.0' does not have a release() function, it is broken and must be fixed. JD> Modules linked in: i2c_mux i2c_i801(-) jc42 snd_pcm_oss JD> snd_mixer_oss snd_seq w83795 snd_seq_device coretemp edd nfsd lockd JD> nfs_acl auth_rpcgss sunrpc cpufreq_conservative cpufreq_userspace JD> cpufreq_powersave acpi_cpufreq mperf fuse nls_utf8 loop dm_mod JD> mt2060 dvb_usb_dib0700 snd_hda_codec_hdmi dib7000p JD> snd_hda_codec_realtek dib0090 dib7000m dib0070 snd_hda_intel JD> ir_lirc_codec dvb_usb snd_hda_codec lirc_dev dib8000 ir_sony_decoder JD> ir_jvc_decoder dvb_core snd_hwdep ir_rc6_decoder ir_rc5_decoder JD> snd_pcm dib3000mc ir_nec_decoder rc_core snd_timer ioatdma e1000e JD> snd iTCO_wdt dibx000_common sg iTCO_vendor_support i7core_edac JD> soundcore i2c_tiny_usb i801_gpio edac_core snd_page_alloc dca pcspkr JD> button radeon ttm sd_mod drm_kms_helper drm i2c_algo_bit fan JD> processor ata_generic ata_piix ahci libahci libata sym53c8xx JD> scsi_transport_spi scsi_mod thermal thermal_sys [last unloaded: JD> gpio_i2cmux] JD> Pid: 29946, comm: rmmod Not tainted 2.6.39-rc3-git8 #77 JD> Call Trace: JD> [] warn_slowpath_common+0x7a/0xb0 JD> [] warn_slowpath_fmt+0x41/0x50 JD> [] ? remove_dir+0x36/0x40 JD> [] device_release+0x82/0x90 JD> [] kobject_release+0x8d/0x1d0 JD> [] ? kobject_del+0x80/0x80 JD> [] kref_put+0x37/0x70 JD> [] kobject_put+0x27/0x60 JD> [] put_device+0x12/0x20 JD> [] platform_device_put+0x12/0x20 JD> [] platform_device_unregister+0x19/0x20 So you tweaked the i801 driver to create and register a platform device for gpio-i2cmux which you then forcibly unregister in the i801 remove handler without providing a release() function on the platform device? That's afaik wrong as the platform_device might be in use (E.G. someone has the sysfs entry open or similar, and you need to use platform_device_alloc() which ensures that the device only gets freed when nobody is using it anymore. -- Bye, Peter Korsgaard DISCLAIMER: Unless indicated otherwise, the information contained in this message is privileged and confidential, and is intended only for the use of the addressee(s) named above and others who have been specifically authorized to receive it. If you are not the intended recipient, you are hereby notified that any dissemination, distribution or copying of this message and/or attachments is strictly prohibited. The company accepts no liability for any damage caused by any virus transmitted by this email. Furthermore, the company does not warrant a proper and complete transmission of this information, nor does it accept liability for any delays. If you have received this message in error, please contact the sender and delete the message. Thank you. -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] i2c/busses: Add support for Aeroflex Gaisler I2CMST controller
>>>>> "Jan" == Jan Andersson writes: Hi, Jan> The register field definitions can currently be shared. The register Jan> offsets (steps) are different since the two prescaler registers have Jan> been merged into one register on the i2cmst core. Jan> For V2 I will move the register field defines from i2c-ocores.c to Jan> i2c-ocores.h and include that file in the new driver - unless anyone Jan> objects. Ben, do you really think that's worthwhile? It's just 6 registers with a few bits in 3 of them. -- Bye, Peter Korsgaard -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Using the gpio i2c multiplexer driver
>>>>> "Guenter" == Guenter Roeck writes: Guenter> Hi all, Guenter> I am trying to use the new GPIO based I2C Guenter> multiplexer. Unfortunately, I have an initialization problem Guenter> with it. Guenter> Some time after registering the multiplexer as platform driver, its Guenter> probe function is called. Unfortunately, that does not happen in sync Guenter> with I2C adapter initialization. The GPIO mux probe function is called Guenter> before the parent's (ie the multiplexed I2C adapter) probe function is Guenter> called. As a result, the GPIO mux driver does not find its parent i2c Guenter> adapter, and the probe function aborts with an error. Guenter> Any idea how I I can fix the problem, ie how I can ensure that Guenter> the GPIO mux probe function is only called after its parent Guenter> I2C adapter is initialized ? What i2c bus controller are you using? Are you registering it's platform data very late (E.G. after you register the platform data for gpiomux)? If you do register the platform data in the correct order, things SHOULD work correctly as busses are listed in drivers/i2c/Makefile before muxes, but alternatively you could play with the init order (E.G. use subsys_initcall instead of module_init in the bus driver, see b8680784875 for an example). -- Bye, Peter Korsgaard -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] i2c-ocores: add some device tree documentation
>>>>> "Jonas" == Jonas Bonn writes: Hi, Jonas> PS: Honestly not crazy about the name 'i2c-ocores' either as it Jonas> doesn't match the name of the upstream Verilog project which is Jonas> called, unfortunately, simply 'i2c'. This driver, however, has Jonas> been upstream for a while so I guess we're stuck with it. Yes, it was more or less the best name I could come with. Just calling it i2c.c wouldn't have been any good either. -- Bye, Peter Korsgaard -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv4] i2c: add generic I2C multiplexer using gpio api
>>>>> "Jean" == Jean Delvare writes:
Hi,
Jean> I've fixed two remaining minor things myself, and your patch is
Jean> ready to be applied now, except for one issue I just noticed. See
Jean> below.
Great.
>> +gpio-i2cmux uses the platform bus, so you need to provide a struct
>> +platform_device with the platform_data pointing to a struct
>> +gpio_i2cmux_platform_data with the I2C adapter number of the master
>> +bus, the number bus segments to create and the GPIO pins used
Jean> "of" got lost in the battle, I've added it back.
Ahh yes, thanks.
>> + for (i = 0; i < pdata->n_gpios; i++) {
>> + ret = gpio_request(pdata->gpios[i], "gpio-i2cmux");
>> + if (ret)
>> + goto err_request_gpio;
>> + gpio_direction_output(pdata->gpios[i], pdata->idle & (1 << i));
Jean> This looks wrong if pdata->idle == GPIO_I2CMUX_NO_IDLE. I think we want
Jean> something along the lines of:
Jean> unsigned initial_state;
Jean> initial_state = pdata->idle == GPIO_I2CMUX_NO_IDLE ?
Jean> pdata->values[0] : pdata->idle;
Jean> (...)
Jean> gpio_direction_output(pdata->gpios[i], initial_state & (1 <<
i));
Jean> What do you think? An alternative is to leave the direction
Jean> uninitialized and hope it's already OK, but I'm not sure how realistic
Jean> this is.
Yeah, going for values[0] is probably the most sensible thing to do if
we don't have an idle state. Relying on the firmware to setup the right
direction in advance is imho not that nice.
Will you do the change yourself, or should I resend the patch?
>> + if (pdata->idle != GPIO_I2CMUX_NO_IDLE)
>> + deselect = gpiomux_deselect;
We could probably move this up and add the assignment of init_state to
this contional, instead of testing against GPIO_I2CMUX_NO_IDLE twice.
--
Bye, Peter Korsgaard
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv4] i2c: add generic I2C multiplexer using gpio api
Add an i2c mux driver providing access to i2c bus segments using a
hardware MUX sitting on a master bus and controlled through gpio pins.
E.G. something like:
-- -- Bus segment 1 - - - - -
| | SCL/SDA| |-- | |
| || |
| || | Bus segment 2 | |
| Linux | GPIO 1..N | MUX|--- Devices
| || | | |
| || | Bus segment M
| || |---| |
-- -- - - - - -
SCL/SDA of the master I2C bus is multiplexed to bus segment 1..M
according to the settings of the GPIO pins 1..N.
Signed-off-by: Peter Korsgaard
---
Changes since v3:
- Adjust according to Jean's 2nd comments
Changes since v2:
- Adjust according to Jean's comments, rename to gpio-i2cmux
Changes since v1:
- Use i2c-mux infrastructure
- Move to drivers/i2c/muxes
Documentation/i2c/muxes/gpio-i2cmux | 65 +
MAINTAINERS |8 ++
drivers/i2c/muxes/Kconfig | 12 +++
drivers/i2c/muxes/Makefile |1 +
drivers/i2c/muxes/gpio-i2cmux.c | 178 +++
include/linux/gpio-i2cmux.h | 38
6 files changed, 302 insertions(+), 0 deletions(-)
create mode 100644 Documentation/i2c/muxes/gpio-i2cmux
create mode 100644 drivers/i2c/muxes/gpio-i2cmux.c
create mode 100644 include/linux/gpio-i2cmux.h
diff --git a/Documentation/i2c/muxes/gpio-i2cmux
b/Documentation/i2c/muxes/gpio-i2cmux
new file mode 100644
index 000..1559103
--- /dev/null
+++ b/Documentation/i2c/muxes/gpio-i2cmux
@@ -0,0 +1,65 @@
+Kernel driver gpio-i2cmux
+
+Author: Peter Korsgaard
+
+Description
+---
+
+gpio-i2cmux is an i2c mux driver providing access to I2C bus segments
+from a master I2C bus and a hardware MUX controlled through GPIO pins.
+
+E.G.:
+
+ -- -- Bus segment 1 - - - - -
+ | | SCL/SDA| |-- | |
+ | || |
+ | || | Bus segment 2 | |
+ | Linux | GPIO 1..N | MUX|--- Devices
+ | || | | |
+ | || | Bus segment M
+ | || |---| |
+ -- -- - - - - -
+
+SCL/SDA of the master I2C bus is multiplexed to bus segment 1..M
+according to the settings of the GPIO pins 1..N.
+
+Usage
+-
+
+gpio-i2cmux uses the platform bus, so you need to provide a struct
+platform_device with the platform_data pointing to a struct
+gpio_i2cmux_platform_data with the I2C adapter number of the master
+bus, the number bus segments to create and the GPIO pins used
+to control it. See include/linux/gpio-i2cmux.h for details.
+
+E.G. something like this for a MUX providing 4 bus segments
+controlled through 3 GPIO pins:
+
+#include
+#include
+
+static const unsigned myboard_gpiomux_gpios[] = {
+ AT91_PIN_PC26, AT91_PIN_PC25, AT91_PIN_PC24
+};
+
+static const unsigned myboard_gpiomux_values[] = {
+ 0, 1, 2, 3
+};
+
+static struct gpio_i2cmux_platform_data myboard_i2cmux_data = {
+ .parent = 1,
+ .base_nr= 2, /* optional */
+ .values = myboard_gpiomux_values,
+ .n_values = ARRAY_SIZE(myboard_gpiomux_values),
+ .gpios = myboard_gpiomux_gpios,
+ .n_gpios= ARRAY_SIZE(myboard_gpiomux_gpios),
+ .idle = 4, /* optional */
+};
+
+static struct platform_device myboard_i2cmux = {
+ .name = "gpio-i2cmux",
+ .id = 0,
+ .dev= {
+ .platform_data = &myboard_i2cmux_data,
+ },
+};
diff --git a/MAINTAINERS b/MAINTAINERS
index b3be8b3..0a53b23 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2579,6 +2579,14 @@ S: Supported
F: drivers/i2c/busses/i2c-gpio.c
F: include/linux/i2c-gpio.h
+GENERIC GPIO I2C MULTIPLEXER DRIVER
+M: Peter Korsgaard
+L: linux-i2c@vger.kernel.org
+S: Supported
+F: drivers/i2c/muxes/gpio-i2cmux.c
+F: include/linux/gpio-i2cmux.h
+F: Documentation/i2c/muxes/gpio-i2cmux
+
GENERIC HDLC (WAN) DRIVERS
M: Krzysztof Halasa
W: http://www.kernel.org/pub/linux/utils/net/hdlc/
diff --git a/drivers/i2c/muxes/Kconfig b/drivers/i2c/muxes/Kconfig
index 4d91d80..90b7a01 100644
--- a/drivers/i2c/muxes/Kconfig
+++ b/drivers/i2c/muxes/Kconfig
@@ -5,6 +5,18 @@
menu "Multiplexer I2C Chip support"
depends on I2C_MUX
+config I2C_MUX_GPIO
+ tristate "GPIO-based I2C multiplexer"
+ depends on GENERIC_GPIO
+ help
+ If
Re: [PATCHv3] i2c: add generic I2C multiplexer using gpio api
>>>>> "Jean" == Jean Delvare writes:
Jean> Hi Peter,
Jean> Thanks for the updated patch.
You're welcome. Thanks for the feedback.
>> SCL/SDA of the master I2C bus is multiplexed to virtual bus 1..M
>> according to the settings of the GPIO pins 1..N.
Jean> Again: these are not virtual buses. These are bus segments behind a
Jean> mux, and they are as real as the trunk segment in front of the mux.
Ok, I renamed every instance of 'virtual bus' with 'bus segment'.
Jean> I'm eager to give it a try on my own PC system, where the SMBus
Jean> is multiplexed using ICH10 GPIO pins. Unfortunately I don't think
Jean> there is a driver for the Intel ICH GPIO pins, so I'll have to
Jean> write one. And then I'll have to write some glue code to
Jean> instantiate the relevant platform device on my system. I'll let
Jean> you know when I get there.
Nice!
>> +static struct gpio_i2cmux_platform_data myboard_i2cmux_data = {
>> + .parent = 1,
>> + .base_nr= 2, /* optional */
>> + .values = myboard_gpiomux_values,
>> + .n_values = ARRAY_SIZE(myboard_gpiomux_values),
>> + .gpio = myboard_gpiomux_gpios,
Jean> .gpios
>> + .gpios = ARRAY_SIZE(myboard_gpiomux_gpios),
Jean> .n_gpios
Ups, fixed.
>> +GENERIC GPIO I2C MULTIPLEXER DRIVER
>> +M: Peter Korsgaard
>> +L: linux-i2c@vger.kernel.org
>> +S: Supported
>> +F: drivers/i2c/muxes/gpio-i2cmux.c
>> +F: include/linux/gpio-i2mux.h
Jean> Typo: gpio-i2cmux.h. And you're missing:
Jean> F: Documentation/i2c/muxes/gpio-i2cmux
Fixed.
>> + config I2C_MUX_GPIO
>> + tristate "GPIO-based I2C multiplexer"
>> + depends on GENERIC_GPIO
>> + help
>> + If you say yes to this option, support will be included for a
>> + GPIO based I2C multiplexer. This driver provides access to
>> + I2C busses connected through a MUX, which is controlled
>> + through GPIO pins.
>> +
>> + This driver can also be built as a module. If so, the module
>> + will be called gpio-i2cmux.
>> +
Jean> Alphabetic order -> goes at the beginning of the list.
>> +obj-$(CONFIG_I2C_MUX_GPIO) += gpio-i2cmux.o
Jean> Alphabetic order -> goes at the beginning of the list.
Fixed.
>> + for (i = 0; i < pdata->n_values; i++) {
>> + u32 nr = pdata->base_nr ? (pdata->base_nr + i) : 0;
>> + bool has_idle = (pdata->idle != (unsigned)-1);
Jean> has_idle doesn't depend on i, so it would be more efficiently set
Jean> outside the loop. You could even store a function pointer instead of a
Jean> boolean value, to be even more efficient.
Jean> BTW, would it make sense to
Jean> #define GPIO_I2CMUX_NO_IDLE ((unsigned)-1)
Jean> to avoid arbitrary cast and constant in the code?
Ok, added define to platform header, and changed the probe code to use
a function pointer instead.
Will send v4 shortly - Thanks for the review.
--
Bye, Peter Korsgaard
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv3] i2c: add generic I2C multiplexer using gpio api
Add an i2c mux driver providing virtual i2c busses using a hardware MUX
sitting on a master bus and controlled through gpio pins.
E.G. something like:
-- -- Virtual bus 1 - - - - -
| | SCL/SDA| |-- | |
| || |
| || | Virtual bus 2 | |
| Linux | GPIO 1..N | MUX|--- Devices
| || | | |
| || | Virtual bus M
| || |---| |
-- -- - - - - -
SCL/SDA of the master I2C bus is multiplexed to virtual bus 1..M
according to the settings of the GPIO pins 1..N.
Signed-off-by: Peter Korsgaard
---
Changes since v2:
- Adjust according to Jean's comments, rename to gpio-i2cmux
Changes since v1:
- Use i2c-mux infrastructure
- Move to drivers/i2c/muxes
Documentation/i2c/muxes/gpio-i2cmux | 65 +
MAINTAINERS |7 ++
drivers/i2c/muxes/Kconfig | 12 +++
drivers/i2c/muxes/Makefile |1 +
drivers/i2c/muxes/gpio-i2cmux.c | 175 +++
include/linux/gpio-i2cmux.h | 35 +++
6 files changed, 295 insertions(+), 0 deletions(-)
create mode 100644 Documentation/i2c/muxes/gpio-i2cmux
create mode 100644 drivers/i2c/muxes/gpio-i2cmux.c
create mode 100644 include/linux/gpio-i2cmux.h
diff --git a/Documentation/i2c/muxes/gpio-i2cmux
b/Documentation/i2c/muxes/gpio-i2cmux
new file mode 100644
index 000..a1768d3
--- /dev/null
+++ b/Documentation/i2c/muxes/gpio-i2cmux
@@ -0,0 +1,65 @@
+Kernel driver gpio-i2cmux
+
+Author: Peter Korsgaard
+
+Description
+---
+
+gpio-i2cmux is an i2c mux driver providing virtual I2C busses from a
+master I2C bus and a hardware MUX controlled through GPIO pins.
+
+E.G.:
+
+ -- -- Virtual bus 1 - - - - -
+ | | SCL/SDA| |-- | |
+ | || |
+ | || | Virtual bus 2 | |
+ | Linux | GPIO 1..N | MUX|--- Devices
+ | || | | |
+ | || | Virtual bus M
+ | || |---| |
+ -- -- - - - - -
+
+SCL/SDA of the master I2C bus is multiplexed to virtual bus 1..M
+according to the settings of the GPIO pins 1..N.
+
+Usage
+-
+
+gpio-i2cmux uses the platform bus, so you need to provide a struct
+platform_device with the platform_data pointing to a struct
+gpio_i2cmux_platform_data with the I2C adapter number of the master
+bus, the number of virtual busses to create and the GPIO pins used
+to control it. See include/linux/gpio-i2cmux.h for details.
+
+E.G. something like this for a MUX providing 4 virtual busses
+controlled through 3 GPIO pins:
+
+#include
+#include
+
+static const unsigned myboard_gpiomux_gpios[] = {
+ AT91_PIN_PC26, AT91_PIN_PC25, AT91_PIN_PC24
+};
+
+static const unsigned myboard_gpiomux_values[] = {
+ 0, 1, 2, 3
+};
+
+static struct gpio_i2cmux_platform_data myboard_i2cmux_data = {
+ .parent = 1,
+ .base_nr= 2, /* optional */
+ .values = myboard_gpiomux_values,
+ .n_values = ARRAY_SIZE(myboard_gpiomux_values),
+ .gpio = myboard_gpiomux_gpios,
+ .gpios = ARRAY_SIZE(myboard_gpiomux_gpios),
+ .idle = 4, /* optional */
+};
+
+static struct platform_device myboard_i2cmux = {
+ .name = "gpio-i2cmux",
+ .id = 0,
+ .dev= {
+ .platform_data = &myboard_i2cmux_data,
+ },
+};
diff --git a/MAINTAINERS b/MAINTAINERS
index b3be8b3..8117a3f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2579,6 +2579,13 @@ S: Supported
F: drivers/i2c/busses/i2c-gpio.c
F: include/linux/i2c-gpio.h
+GENERIC GPIO I2C MULTIPLEXER DRIVER
+M: Peter Korsgaard
+L: linux-i2c@vger.kernel.org
+S: Supported
+F: drivers/i2c/muxes/gpio-i2cmux.c
+F: include/linux/gpio-i2mux.h
+
GENERIC HDLC (WAN) DRIVERS
M: Krzysztof Halasa
W: http://www.kernel.org/pub/linux/utils/net/hdlc/
diff --git a/drivers/i2c/muxes/Kconfig b/drivers/i2c/muxes/Kconfig
index 4d91d80..394a8b1 100644
--- a/drivers/i2c/muxes/Kconfig
+++ b/drivers/i2c/muxes/Kconfig
@@ -25,4 +25,16 @@ config I2C_MUX_PCA954x
This driver can also be built as a module. If so, the module
will be called pca954x.
+ config I2C_MUX_GPIO
+ tristate "GPIO-based I2C multiplexer"
+ depends on GENERIC_GPIO
+ help
+ If you say yes to this option, support will be included for a
+
Re: [PATCHv2,resend] i2c: add generic I2C multiplexer using gpio api
>>>>> "Jean" == Jean Delvare writes:
Jean> Hi Peter,
Jean> Sorry and the late answer.
Thanks for the feedback, see below for a few comments.
>> +++ b/Documentation/i2c/busses/i2c-gpiomux
>> @@ -0,0 +1,65 @@
>> +Kernel driver i2c-gpiomux
>> +
>> +Author: Peter Korsgaard
>> +
>> +Description
>> +---
>> +
>> +i2c-gpiomux is an i2c bus driver providing virtual I2C busses from a
Jean> It is an i2c mux driver now.
Fixed.
>> +Usage
>> +-
>> +
>> +i2c-gpiomux uses the platform bus, so you need to provide a struct
>> +platform_device with the platform_data pointing to a struct
>> +gpiomux_i2c_platform_data with the I2C adapter number of the master
Jean> The structure should be named i2c_gpiomux_platform_data, so that the
Jean> prefix matches the driver name (but see below).
Fixed - It's now gpio_i2cmux_platform_data.
>> +bus, the number of virtual busses to create and the GPIO pins used
>> +to control it. See include/linux/i2c-gpiomux.h for details.
>> +
>> +E.G. something like this for a MUX providing 4 virtual busses
>> +controlled through 3 GPIO pins:
>> +
>> +#include
>> +#include
>> +
>> +static unsigned myboard_gpiomux_pins[] = {
>> + AT91_PIN_PC26, AT91_PIN_PC25, AT91_PIN_PC24
>> +};
>> +
>> +static unsigned myboard_gpiomux_values[] = {
>> + 0, 1, 2, 3
>> +};
>> +
>> +static struct gpiomux_i2c_platform_data myboard_i2cmux_data = {
>> + .parent = 1,
>> + .base_nr= 2,
Jean> There might be no need to specify the numbers of the child I2C segments.
Jean> Your driver should handle the case where base_nr isn't set, and not ask
Jean> for specific i2c_adpater numbers then.
Base_nr is now optional.
>> + .busses = ARRAY_SIZE(myboard_gpiomux_values),
>> + .gpios = ARRAY_SIZE(myboard_gpiomux_pins),
>> + .gpio = myboard_gpiomux_pins,
>> + .values = myboard_gpiomux_values,
Jean> I find your naming convention (or lack thereof) confusing. What about:
Jean> .values = myboard_gpiomux_values,
Jean> .n_values = ARRAY_SIZE(myboard_gpiomux_values),
Jean> .pins = myboard_gpiomux_pins,
Jean> .n_pins = ARRAY_SIZE(myboard_gpiomux_pins),
Jean> This would be more consistent and obvious.
Ok, renamed. I'm using gpios/n_gpios instead though, as that is what
they are.
>> + .idle = 4,
Jean> This should be optional. Not all hardware setup can actually disable
Jean> all children.
I've made idle == (unsigned)-1 signal no idle support (cannot use 0, as
that's a fairly common idle value).
>> +};
>> +
>> +static struct platform_device myboard_i2cmux = {
>> + .name = "i2c-gpiomux",
>> + .id = 0,
>> + .dev= {
>> + .platform_data = &myboard_i2cmux_data,
>> + },
>> +};
Jean> All these structures can presumably be marked const.
Platform_device and the main myboard_i2cmux_data cannot as the API
specifies non const, so you get compiler warnings, but the lowe level
values and gpio arrays can (and I've done so).
>> + config I2C_GPIOMUX
>> + tristate "GPIO-based I2C multiplexer"
>> + depends on GENERIC_GPIO
>> + help
>> + If you say yes to this option, support will be included for a
>> + GPIO based I2C multiplexer. This driver provides virtual I2C
>> + busses from a master bus through a MUX controlled through
Jean> There is nothing virtual about these bus segments. They are very real.
Reworded.
>> obj-$(CONFIG_I2C_MUX_PCA9541) += pca9541.o
>> obj-$(CONFIG_I2C_MUX_PCA954x) += pca954x.o
>> +obj-$(CONFIG_I2C_GPIOMUX) += i2c-gpiomux.o
Jean> It should be obvious from the above that your config option should be
Jean> named CONFIG_I2C_MUX_GPIO.
Renamed.
Jean> Not sure about the driver name. i2c-gpiomux makes it look like an i2c
core or
Jean> i2c bus driver, which it is not. Maybe gpio-i2cmux would be better? I
Jean> see we already have drivers named gpio-fan for fans controlled over
Jean> GPIOs, as well as gpio_mouse and gpio_keys.
Renamed to gpio-i2cmux.
>> +static void gpiomux_set(struct gpiomux_i2c *i2c, unsigned val)
Jean> i2c could be a const pointer.
Done.
>> +{
>> + int i;
>> +
>> + for (i = 0; i < i2c->data.gpios; i++)
Jean> Double space after the first ";".
>> + gpio_set_value(i2c->data.gpio[i], val & (1< Although check
[PATCHv2,resend] i2c: add generic I2C multiplexer using gpio api
From: Peter Korsgaard
Add an i2c mux driver providing virtual i2c busses using a hardware MUX
sitting on a master bus and controlled through gpio pins.
E.G. something like:
-- -- Virtual bus 1 - - - - -
| | SCL/SDA| |-- | |
| || |
| || | Virtual bus 2 | |
| Linux | GPIO 1..N | MUX|--- Devices
| || | | |
| || | Virtual bus M
| || |---| |
-- -- - - - - -
SCL/SDA of the master I2C bus is multiplexed to virtual bus 1..M
according to the settings of the GPIO pins 1..N.
Signed-off-by: Peter Korsgaard
---
Changes since v1:
- Use i2c-mux infrastructure
- Move to drivers/i2c/muxes
Documentation/i2c/busses/i2c-gpiomux | 65 +
MAINTAINERS |7 ++
drivers/i2c/muxes/Kconfig| 12 +++
drivers/i2c/muxes/Makefile |1 +
drivers/i2c/muxes/i2c-gpiomux.c | 172 ++
include/linux/i2c-gpiomux.h | 35 +++
6 files changed, 292 insertions(+), 0 deletions(-)
create mode 100644 Documentation/i2c/busses/i2c-gpiomux
create mode 100644 drivers/i2c/muxes/i2c-gpiomux.c
create mode 100644 include/linux/i2c-gpiomux.h
diff --git a/Documentation/i2c/busses/i2c-gpiomux
b/Documentation/i2c/busses/i2c-gpiomux
new file mode 100644
index 000..b0a7746
--- /dev/null
+++ b/Documentation/i2c/busses/i2c-gpiomux
@@ -0,0 +1,65 @@
+Kernel driver i2c-gpiomux
+
+Author: Peter Korsgaard
+
+Description
+---
+
+i2c-gpiomux is an i2c bus driver providing virtual I2C busses from a
+master I2C bus and a hardware MUX controlled through GPIO pins.
+
+E.G.:
+
+ -- -- Virtual bus 1 - - - - -
+ | | SCL/SDA| |-- | |
+ | || |
+ | || | Virtual bus 2 | |
+ | Linux | GPIO 1..N | MUX|--- Devices
+ | || | | |
+ | || | Virtual bus M
+ | || |---| |
+ -- -- - - - - -
+
+SCL/SDA of the master I2C bus is multiplexed to virtual bus 1..M
+according to the settings of the GPIO pins 1..N.
+
+Usage
+-
+
+i2c-gpiomux uses the platform bus, so you need to provide a struct
+platform_device with the platform_data pointing to a struct
+gpiomux_i2c_platform_data with the I2C adapter number of the master
+bus, the number of virtual busses to create and the GPIO pins used
+to control it. See include/linux/i2c-gpiomux.h for details.
+
+E.G. something like this for a MUX providing 4 virtual busses
+controlled through 3 GPIO pins:
+
+#include
+#include
+
+static unsigned myboard_gpiomux_pins[] = {
+ AT91_PIN_PC26, AT91_PIN_PC25, AT91_PIN_PC24
+};
+
+static unsigned myboard_gpiomux_values[] = {
+ 0, 1, 2, 3
+};
+
+static struct gpiomux_i2c_platform_data myboard_i2cmux_data = {
+ .parent = 1,
+ .base_nr= 2,
+ .busses = ARRAY_SIZE(myboard_gpiomux_values),
+ .gpios = ARRAY_SIZE(myboard_gpiomux_pins),
+ .gpio = myboard_gpiomux_pins,
+ .values = myboard_gpiomux_values,
+ .idle = 4,
+};
+
+static struct platform_device myboard_i2cmux = {
+ .name = "i2c-gpiomux",
+ .id = 0,
+ .dev= {
+ .platform_data = &myboard_i2cmux_data,
+ },
+};
diff --git a/MAINTAINERS b/MAINTAINERS
index 0094224..ffe181a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2567,6 +2567,13 @@ S: Supported
F: drivers/i2c/busses/i2c-gpio.c
F: include/linux/i2c-gpio.h
+GENERIC GPIO I2C MULTIPLEXER DRIVER
+M: Peter Korsgaard
+L: linux-i2c@vger.kernel.org
+S: Supported
+F: drivers/i2c/muxes/i2c-gpiomux.c
+F: include/linux/i2c-gpiomux.h
+
GENERIC HDLC (WAN) DRIVERS
M: Krzysztof Halasa
W: http://www.kernel.org/pub/linux/utils/net/hdlc/
diff --git a/drivers/i2c/muxes/Kconfig b/drivers/i2c/muxes/Kconfig
index 4d91d80..0345e37 100644
--- a/drivers/i2c/muxes/Kconfig
+++ b/drivers/i2c/muxes/Kconfig
@@ -25,4 +25,16 @@ config I2C_MUX_PCA954x
This driver can also be built as a module. If so, the module
will be called pca954x.
+ config I2C_GPIOMUX
+ tristate "GPIO-based I2C multiplexer"
+ depends on GENERIC_GPIO
+ help
+ If you say yes to this option, support will be included for a
+ GPIO based I2C multiplexer. This driver provides virtual I2C
+ busses from a master bus
[PATCHv2] i2c: add generic I2C multiplexer using gpio api
From: Peter Korsgaard
Add an i2c mux driver providing virtual i2c busses using a hardware MUX
sitting on a master bus and controlled through gpio pins.
E.G. something like:
-- -- Virtual bus 1 - - - - -
| | SCL/SDA| |-- | |
| || |
| || | Virtual bus 2 | |
| Linux | GPIO 1..N | MUX|--- Devices
| || | | |
| || | Virtual bus M
| || |---| |
-- -- - - - - -
SCL/SDA of the master I2C bus is multiplexed to virtual bus 1..M
according to the settings of the GPIO pins 1..N.
Signed-off-by: Peter Korsgaard
---
Changes since v1:
- Use i2c-mux infrastructure
- Move to drivers/i2c/muxes
Documentation/i2c/busses/i2c-gpiomux | 65 +
MAINTAINERS |7 ++
drivers/i2c/muxes/Kconfig| 12 +++
drivers/i2c/muxes/Makefile |1 +
drivers/i2c/muxes/i2c-gpiomux.c | 172 ++
include/linux/i2c-gpiomux.h | 35 +++
6 files changed, 292 insertions(+), 0 deletions(-)
create mode 100644 Documentation/i2c/busses/i2c-gpiomux
create mode 100644 drivers/i2c/muxes/i2c-gpiomux.c
create mode 100644 include/linux/i2c-gpiomux.h
diff --git a/Documentation/i2c/busses/i2c-gpiomux
b/Documentation/i2c/busses/i2c-gpiomux
new file mode 100644
index 000..b0a7746
--- /dev/null
+++ b/Documentation/i2c/busses/i2c-gpiomux
@@ -0,0 +1,65 @@
+Kernel driver i2c-gpiomux
+
+Author: Peter Korsgaard
+
+Description
+---
+
+i2c-gpiomux is an i2c bus driver providing virtual I2C busses from a
+master I2C bus and a hardware MUX controlled through GPIO pins.
+
+E.G.:
+
+ -- -- Virtual bus 1 - - - - -
+ | | SCL/SDA| |-- | |
+ | || |
+ | || | Virtual bus 2 | |
+ | Linux | GPIO 1..N | MUX|--- Devices
+ | || | | |
+ | || | Virtual bus M
+ | || |---| |
+ -- -- - - - - -
+
+SCL/SDA of the master I2C bus is multiplexed to virtual bus 1..M
+according to the settings of the GPIO pins 1..N.
+
+Usage
+-
+
+i2c-gpiomux uses the platform bus, so you need to provide a struct
+platform_device with the platform_data pointing to a struct
+gpiomux_i2c_platform_data with the I2C adapter number of the master
+bus, the number of virtual busses to create and the GPIO pins used
+to control it. See include/linux/i2c-gpiomux.h for details.
+
+E.G. something like this for a MUX providing 4 virtual busses
+controlled through 3 GPIO pins:
+
+#include
+#include
+
+static unsigned myboard_gpiomux_pins[] = {
+ AT91_PIN_PC26, AT91_PIN_PC25, AT91_PIN_PC24
+};
+
+static unsigned myboard_gpiomux_values[] = {
+ 0, 1, 2, 3
+};
+
+static struct gpiomux_i2c_platform_data myboard_i2cmux_data = {
+ .parent = 1,
+ .base_nr= 2,
+ .busses = ARRAY_SIZE(myboard_gpiomux_values),
+ .gpios = ARRAY_SIZE(myboard_gpiomux_pins),
+ .gpio = myboard_gpiomux_pins,
+ .values = myboard_gpiomux_values,
+ .idle = 4,
+};
+
+static struct platform_device myboard_i2cmux = {
+ .name = "i2c-gpiomux",
+ .id = 0,
+ .dev= {
+ .platform_data = &myboard_i2cmux_data,
+ },
+};
diff --git a/MAINTAINERS b/MAINTAINERS
index 7679bf3..f63d518 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2550,6 +2550,13 @@ S: Supported
F: drivers/i2c/busses/i2c-gpio.c
F: include/linux/i2c-gpio.h
+GENERIC GPIO I2C MULTIPLEXER DRIVER
+M: Peter Korsgaard
+L: linux-i2c@vger.kernel.org
+S: Supported
+F: drivers/i2c/muxes/i2c-gpiomux.c
+F: include/linux/i2c-gpiomux.h
+
GENERIC HDLC (WAN) DRIVERS
M: Krzysztof Halasa
W: http://www.kernel.org/pub/linux/utils/net/hdlc/
diff --git a/drivers/i2c/muxes/Kconfig b/drivers/i2c/muxes/Kconfig
index 4c9a99c..d9e987d 100644
--- a/drivers/i2c/muxes/Kconfig
+++ b/drivers/i2c/muxes/Kconfig
@@ -15,4 +15,16 @@ config I2C_MUX_PCA954x
This driver can also be built as a module. If so, the module
will be called pca954x.
+ config I2C_GPIOMUX
+ tristate "GPIO-based I2C multiplexer"
+ depends on GENERIC_GPIO
+ help
+ If you say yes to this option, support will be included for a
+ GPIO based I2C multiplexer. This driver provides virtual I2C
+ busses from a master bus
Re: [PATCH] i2c: add generic I2C multiplexer using gpio api
>>>>> "Michael" == Michael Lawnick writes: >> But this patch is independent from that work as the mux access isn't >> through I2C, hence no changes to i2c-core needed. Michael> i2c-mux patch does not expect path control via i2c. Michael> Your scenario fits perfectly. Hmm, I'll take a closer look at the last version then - sorry. In the past it afaik did. What is the point of the i2c-core changes if path control isn't via i2c? -- Bye, Peter Korsgaard -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] i2c: add generic I2C multiplexer using gpio api
>>>>> "Jean" == Jean Delvare writes: Jean> The problem is exactly that your patch is independent. It _should_ Jean> build on top of Michael's work, to avoid redundancy. Why? We've discussed this for years, E.G.: http://www.mail-archive.com/i...@lm-sensors.org/msg01539.html None of the locking changes in core are needed when the MUX isn't controlled through I2C. I guess I could use the i2c-mux.c functions, as there's some overlap in the trivial gpiomux_xfer/gpiomux_func functions, but that can easily be done once the mux stuff is merged. -- Bye, Peter Korsgaard -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] i2c: add generic I2C multiplexer using gpio api
>>>>> "Jean" == Jean Delvare writes: Hi, Jean> Michael and myself are in the process of adding core support for Jean> bus multiplexing to the i2c subsystem. There is no point in Jean> reviewing more specific implementations until the core part is Jean> merged (which is scheduled to happen in kernel 2.6.36.) But this patch is independent from that work as the mux access isn't through I2C, hence no changes to i2c-core needed. Do you want me to resend or do you still have the original patch? -- Bye, Peter Korsgaard -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] i2c: add generic I2C multiplexer using gpio api
>>>>> "Peter" == Peter Korsgaard writes: Peter> Add an i2c bus driver providing virtual i2c busses using a hardware Peter> MUX sitting on a master bus and controlled through gpio pins. Peter> E.G. something like: Peter> -- -- Virtual bus 1 - - - - - Peter> | | SCL/SDA| |-- | | Peter> | || | Peter> | || | Virtual bus 2 | | Peter> | Linux | GPIO 1..N | MUX|--- Devices Peter> | || | | | Peter> | || | Virtual bus M Peter> | || |---| | Peter> -- -- - - - - - Peter> SCL/SDA of the master I2C bus is multiplexed to virtual bus 1..M Peter> according to the settings of the GPIO pins 1..N. Peter> Signed-off-by: Peter Korsgaard Comments? -- Bye, Peter Korsgaard -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] i2c: add generic I2C multiplexer using gpio api
Add an i2c bus driver providing virtual i2c busses using a hardware MUX
sitting on a master bus and controlled through gpio pins.
E.G. something like:
-- -- Virtual bus 1 - - - - -
| | SCL/SDA| |-- | |
| || |
| || | Virtual bus 2 | |
| Linux | GPIO 1..N | MUX|--- Devices
| || | | |
| || | Virtual bus M
| || |---| |
-- -- - - - - -
SCL/SDA of the master I2C bus is multiplexed to virtual bus 1..M
according to the settings of the GPIO pins 1..N.
Signed-off-by: Peter Korsgaard
---
Documentation/i2c/busses/i2c-gpiomux | 65 +++
MAINTAINERS |7 ++
drivers/i2c/busses/Kconfig | 12 ++
drivers/i2c/busses/Makefile |1 +
drivers/i2c/busses/i2c-gpiomux.c | 196 ++
include/linux/i2c-gpiomux.h | 35 ++
6 files changed, 316 insertions(+), 0 deletions(-)
create mode 100644 Documentation/i2c/busses/i2c-gpiomux
create mode 100644 drivers/i2c/busses/i2c-gpiomux.c
create mode 100644 include/linux/i2c-gpiomux.h
diff --git a/Documentation/i2c/busses/i2c-gpiomux
b/Documentation/i2c/busses/i2c-gpiomux
new file mode 100644
index 000..b0a7746
--- /dev/null
+++ b/Documentation/i2c/busses/i2c-gpiomux
@@ -0,0 +1,65 @@
+Kernel driver i2c-gpiomux
+
+Author: Peter Korsgaard
+
+Description
+---
+
+i2c-gpiomux is an i2c bus driver providing virtual I2C busses from a
+master I2C bus and a hardware MUX controlled through GPIO pins.
+
+E.G.:
+
+ -- -- Virtual bus 1 - - - - -
+ | | SCL/SDA| |-- | |
+ | || |
+ | || | Virtual bus 2 | |
+ | Linux | GPIO 1..N | MUX|--- Devices
+ | || | | |
+ | || | Virtual bus M
+ | || |---| |
+ -- -- - - - - -
+
+SCL/SDA of the master I2C bus is multiplexed to virtual bus 1..M
+according to the settings of the GPIO pins 1..N.
+
+Usage
+-
+
+i2c-gpiomux uses the platform bus, so you need to provide a struct
+platform_device with the platform_data pointing to a struct
+gpiomux_i2c_platform_data with the I2C adapter number of the master
+bus, the number of virtual busses to create and the GPIO pins used
+to control it. See include/linux/i2c-gpiomux.h for details.
+
+E.G. something like this for a MUX providing 4 virtual busses
+controlled through 3 GPIO pins:
+
+#include
+#include
+
+static unsigned myboard_gpiomux_pins[] = {
+ AT91_PIN_PC26, AT91_PIN_PC25, AT91_PIN_PC24
+};
+
+static unsigned myboard_gpiomux_values[] = {
+ 0, 1, 2, 3
+};
+
+static struct gpiomux_i2c_platform_data myboard_i2cmux_data = {
+ .parent = 1,
+ .base_nr= 2,
+ .busses = ARRAY_SIZE(myboard_gpiomux_values),
+ .gpios = ARRAY_SIZE(myboard_gpiomux_pins),
+ .gpio = myboard_gpiomux_pins,
+ .values = myboard_gpiomux_values,
+ .idle = 4,
+};
+
+static struct platform_device myboard_i2cmux = {
+ .name = "i2c-gpiomux",
+ .id = 0,
+ .dev= {
+ .platform_data = &myboard_i2cmux_data,
+ },
+};
diff --git a/MAINTAINERS b/MAINTAINERS
index 7642365..f80ad93 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2453,6 +2453,13 @@ S: Supported
F: drivers/i2c/busses/i2c-gpio.c
F: include/linux/i2c-gpio.h
+GENERIC GPIO I2C MULTIPLEXER DRIVER
+M: Peter Korsgaard
+L: linux-i2c@vger.kernel.org
+S: Supported
+F: drivers/i2c/busses/i2c-gpiomux.c
+F: include/linux/i2c-gpiomux.h
+
GENERIC HDLC (WAN) DRIVERS
M: Krzysztof Halasa
W: http://www.kernel.org/pub/linux/utils/net/hdlc/
diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index bceafbf..055b0f1 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -364,6 +364,18 @@ config I2C_GPIO
This is a very simple bitbanging I2C driver utilizing the
arch-neutral GPIO API to control the SCL and SDA lines.
+config I2C_GPIOMUX
+ tristate "GPIO-based I2C multiplexer"
+ depends on GENERIC_GPIO
+ help
+ If you say yes to this option, support will be included for a
+ GPIO based I2C multiplexer. This driver provides virtual I2C
+ busses from a master bus through a MUX controlled through
+ the generic GPIO interface.
+
+
Re: [PATCH RESEND 1/1] i2c: Add support for Xilinx XPS IIC Bus Interface
>>>>> "Wolfram" == Wolfram Sang writes: >> >> + * i2c-xiic.c >> > >> > the directory path to the file would have been nice. >> >> Will update Wolfram> Ben, are you sure? Those filenames (especially with Wolfram> directories) easily go stale when reorganizing the tree. Exactly, that's normally frowned upon in other parts of the tree. -- Bye, Peter Korsgaard -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] i2c-mpc: Do not generate STOP after read.
>>>>> "Joakim" == Joakim Tjernlund writes: Hi, Joakim> Ah, that explains it. Who then will look after i2c-mpc? Kumar? Ben Dooks (embedded i2c maintainer). He's afaik coming home today, so give him a few days to catch up on mails. -- Bye, Peter Korsgaard -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: About a suspicious msleep during doxfer in s3c2410 i2c bus driver
>>>>> "Ben" == Ben Dooks writes: Hi, >> It wasn't for the client devices, it was for issues with >> synchronisation with the controller hardware. I'm just going on >> the basis of recollections of previous conversations with Ben >> here; I'm not sure to what extent this might be an issue with the >> way the hardware works requiring the driver to jump through hoops. Ben> I think this has been in here for a while, and may well not be Ben> necessary any more. If anyone else has tested this without the Ben> msleep() here, then I'd be interested to know. I'll test on my 6410 board over the weekend. Notice that I've been hacking on the uboot drivers/i2c/s3c24x0_i2c.c file to add s3c64xx support, and I there had to add a udelay before setting the start condition to get it to work stable. -- Bye, Peter Korsgaard -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] i2c-s3c2410: s3c24xx_i2c_init: don't clobber IICLC value
>>>>> "Peter" == Peter Korsgaard writes: Peter> s3c24xx_i2c_init() was overwriting the IICLC value set just Peter> above in s3c24xx_i2c_clockrate() with zero, effectively Peter> disabling the platform line control setting. ping? -- Bye, Peter Korsgaard -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] i2c-s3c2410: s3c24xx_i2c_init: don't clobber IICLC value
s3c24xx_i2c_init() was overwriting the IICLC value set just above in s3c24xx_i2c_clockrate() with zero, effectively disabling the platform line control setting. Signed-off-by: Peter Korsgaard --- drivers/i2c/busses/i2c-s3c2410.c |5 - 1 files changed, 0 insertions(+), 5 deletions(-) diff --git a/drivers/i2c/busses/i2c-s3c2410.c b/drivers/i2c/busses/i2c-s3c2410.c index 8f42a45..20bb0ce 100644 --- a/drivers/i2c/busses/i2c-s3c2410.c +++ b/drivers/i2c/busses/i2c-s3c2410.c @@ -763,11 +763,6 @@ static int s3c24xx_i2c_init(struct s3c24xx_i2c *i2c) dev_info(i2c->dev, "bus frequency set to %d KHz\n", freq); dev_dbg(i2c->dev, "S3C2410_IICCON=0x%02lx\n", iicon); - /* check for s3c2440 i2c controller */ - - if (s3c24xx_i2c_is2440(i2c)) - writel(0x0, i2c->regs + S3C2440_IICLC); - return 0; } -- 1.6.3.3 -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/9] I2C: i2c-ocores: Can add I2C devices to the bus
>>>>> "Richard" == Richard Röjfors writes: Richard> There is sometimes a need for the ocores driver to add Richard> devices to the bus when installed. Richard> i2c_register_board_info can not always be used, because the Richard> I2C devices are not known at an early state, they could for Richard> instance be connected on a I2C bus on a PCI device which has Richard> the Open Cores IP. Richard> i2c_new_device can not be used in all cases either since the Richard> resulting bus nummer might be unknown. Richard> The solution is the pass a list of I2C devices in the Richard> platform data to the Open Cores driver. This is useful for Richard> MFD drivers. Richard> Signed-off-by: Richard Röjfors Acked-by: Peter Korsgaard -- Bye, Peter Korsgaard -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] I2C: ocores can add I2C devices to the bus
>>>>> "Jean" == Jean Delvare writes:
Hi,
Jean> I don't like the idea much either, nor the implementation.
Jean> Firstly, I don't understand why this would be needed. I can understand
Jean> that in some cases you don't know the I2C bus number in advance, but
Jean> then some code must still instantiate the I2C bus, and the same code
Jean> should be able to call i2c_new_device() directly to instantiate the
Jean> devices on that bus. Richard, did you try to just do this? If it
Jean> doesn't work, please explain why.
Indeed. Isn't it just a matter of using i2c_add_numbered_adapter -
E.G.:
--- linux-2.6/drivers/i2c/busses/i2c-ocores.c 2008-11-26 11:16:27.0
+0100
+++ linux-2.6-new/drivers/i2c/busses/i2c-ocores.c 2008-12-13
19:59:12.0 +0100
@@ -261,11 +261,12 @@
/* hook up driver to tree */
platform_set_drvdata(pdev, i2c);
i2c->adap = ocores_adapter;
+ i2c->adap.nr = pdev->id;
i2c_set_adapdata(&i2c->adap, i2c);
i2c->adap.dev.parent = &pdev->dev;
/* add i2c adapter to i2c tree */
- ret = i2c_add_adapter(&i2c->adap);
+ ret = i2c_add_numbered_adapter(&i2c->adap);
if (ret) {
dev_err(&pdev->dev, "Failed to add adapter\n");
goto add_adapter_failed;
Or am I misunderstanding the issue?
--
Bye, Peter Korsgaard
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] i2c-mpc: generate START condition after STOP caused by read i2c_msg
>>>>> "Esben" == Esben Haabendal writes: Hi, Esben> It's strange, that line looks perfectly fine when I check the Esben> mail in my GMail inbox and the outbox from the account I sent Esben> it from. Well, it is here and in the archive: http://ozlabs.org/pipermail/linuxppc-dev/2009-May/072274.html Please consider using git send-email for patches. -- Bye, Peter Korsgaard -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] i2c-mpc: generate START condition after STOP caused by read i2c_msg
>>>>> "Esben" == Esben Haabendal writes: Hi, >> I wanted to test it, but it does not apply due to line breaks (check >> @@-line). Also, I don't really have the time to dig into the topic, so I >> would only test it and give a tested-by-tag if it doesn't break anything >> here. I think Joakim would be a good candidate for an acked-by . Esben> I've checked both my copy in my "Sent" folder and the copy Esben> received from the list, and I cannot see any "line break" Esben> breakage of the patch. I guess Wolfram referred to the context line which was clearly word wrapped: @@ -456,17 +456,22 @@ static int mpc_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num) The other lines look fine. -- Bye, Peter Korsgaard -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Again on virtual i2c adapter support.
>>>>> "Michelle" == Michelle Konzack writes: Hi, >> > I have been doing i2c multiplexers where the multiplexers themselves >> > are not controlled through i2c for years without problems. Michelle> So you use those Multiplexer only for level switching where Michelle> even on different ports the Address is unique? This is Michelle> working with the Maxim chips too. No, there can (and there are) multiple chips with the same address. What I mean is simply that the mux is't controlled by writing I2C commands to it, but through something else (in this case a sram-like bus on a fpga). -- Bye, Peter Korsgaard -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Again on virtual i2c adapter support.
>>>>> "Jean" == Jean Delvare writes: >> True. I unsually solve this by making sure the multiplexer starts in >> an unconnected state so the trunk probe doesn't find anything, or >> simply not use the old style probing. Jean> Please keep in mind that the difficulty here is with probing itself, Jean> not just with old-style. The new binding model also has a detection Jean> mode, which is also affected. Making sure that the multiplexer is in an Jean> unconnected state initially isn't sufficient, as you can load I2C chip Jean> drivers at any later point in time and this will trigger a new Jean> detection cycle. And not all multiplexers can be disabled that way, Jean> some have always one outer channel enabled. Yes, but my code (http://www.mail-archive.com/i...@lm-sensors.org/msg01539.html) does: lock parent mutex enable mux do transfer on parent bus disable mux unlock parent So that afaik shouldn't be a problem. If your mux doesn't have an enable control and there's no unused connection you can use instead, then that ofcourse doesn't work. -- Bye, Peter Korsgaard -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Again on virtual i2c adapter support.
>>>>> "Rodolfo" == Rodolfo Giometti writes: Hi, >> I have been doing i2c multiplexers where the multiplexers themselves >> are not controlled through i2c for years without problems. >> >> I afaik even posted an example driver back when Dave posted his >> multiplex hack. Rodolfo> Did you post the code on this list? Yes, but notice that it was for a multiplexer NOT controlled over I2C. http://www.mail-archive.com/i...@lm-sensors.org/msg01539.html -- Bye, Peter Korsgaard -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Again on virtual i2c adapter support.
>>>>> "Jean" == Jean Delvare writes: Hi, Jean> There are still a few caveats when doing this. In particular, Jean> if you use drivers which probe the I2C bus for devices, you Jean> must make sure that the devices will be properly found on Jean> either the trunk or one of the multiplexer branches but not Jean> both. In fact this is with this specific case in mind that I Jean> decided to wait for the i2c device driver binding model to be Jean> cleaned up before going on with full multiplexing support. True. I unsually solve this by making sure the multiplexer starts in an unconnected state so the trunk probe doesn't find anything, or simply not use the old style probing. -- Bye, Peter Korsgaard -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Again on virtual i2c adapter support.
>>>>> "Jean" == Jean Delvare writes: Hi, Jean> Oh, and please stop calling the thing "virtual i2c adapter support". Jean> These adapters are very real. What you are working on is better Jean> described as "i2c bus multiplexing support". Not only that, it's afaik about doing i2c bus multiplexing where the multiplexers are themselves i2c devices - E.G. stuff where you might need to recursively call into the i2c core. I have been doing i2c multiplexers where the multiplexers themselves are not controlled through i2c for years without problems. I afaik even posted an example driver back when Dave posted his multiplex hack. -- Bye, Peter Korsgaard -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
