Re: [PATCH 0/5] i2c-sh_mobile non-urgent changes

2012-11-16 Thread Wolfram Sang
On Wed, Oct 24, 2012 at 07:55:59PM +0900, Shinya Kuribayashi wrote:
 Hello,
 
 This is the first batch to fix the i2c-sh_mobile driver.  As a first
 step, this comprises of the SCL optimization work and a simple fix to
 annoying spurious WAIT interrupt issue; in other words, this does not
 change tx/rx data handling logic.
 
 This driver has an architectural problem with send/receive procedures
 and needs a fundamental overhaul.  I've been working on splitting into
 logical chunks, but not yet completed.
 
 Patches are prepared against the vanilla v3.6.  I don't have a chance
 to give it a try with v3.6+ kernels myself, but tend to be optimistic
 about that.  I have been cooking these patches over a year, and they
 work perfectly fine with older kernels ..v3.4 so far.

Looks good to me, like the extensive patch descriptions. I am going to
apply it to for-next with patches 2+3 squashed, because after patch 2 we
would have a buggy state otherwise. Let me know if you are not okay with
that.

Thanks,

   Wolfram

-- 
Pengutronix e.K.   | Wolfram Sang|
Industrial Linux Solutions | http://www.pengutronix.de/  |


signature.asc
Description: Digital signature


Re: [PATCH 1/2] i2c: mux: Add dt support to i2c-mux-gpio driver

2012-11-16 Thread Wolfram Sang
On Thu, Oct 25, 2012 at 06:23:53PM +0200, Maxime Ripard wrote:
 Allow the i2c-mux-gpio to be used by a device tree enabled device. The
 bindings are inspired by the one found in the i2c-mux-pinctrl driver.
 
 Signed-off-by: Maxime Ripard maxime.rip...@free-electrons.com
 Reviewed-by: Stephen Warren swar...@nvidia.com
 Acked-by: Peter Korsgaard peter.korsga...@barco.com

checkpatch found a whitespace error in line 115, please run it before
submitting. Fixed that and pushed to for-next. Thanks.

-- 
Pengutronix e.K.   | Wolfram Sang|
Industrial Linux Solutions | http://www.pengutronix.de/  |


signature.asc
Description: Digital signature


Re: [PATCH 2/2] ARM: dts: cfa10049: Add the i2c muxer buses to the CFA-10049

2012-11-16 Thread Wolfram Sang
On Thu, Oct 25, 2012 at 06:23:54PM +0200, Maxime Ripard wrote:
 This will allow to add the 3 Nuvoton NAU7802 ADCs and the NXP PCA9555
 GPIO expander eventually.
 
 Signed-off-by: Maxime Ripard maxime.rip...@free-electrons.com

Shawn, I pushed the needed code to my for-next now. So you could apply
this patch for 3.8 or give an ack in case you want me to pick it via
i2c.

Regards,

   Wolfram

-- 
Pengutronix e.K.   | Wolfram Sang|
Industrial Linux Solutions | http://www.pengutronix.de/  |


signature.asc
Description: Digital signature


Re: [PATCH 0/5] i2c-sh_mobile non-urgent changes

2012-11-16 Thread Shinya Kuribayashi
On 11/16/2012 5:07 PM, Wolfram Sang wrote:
 Looks good to me, like the extensive patch descriptions. I am going to
 apply it to for-next with patches 2+3 squashed, because after patch 2 we
 would have a buggy state otherwise. Let me know if you are not okay with
 that.

It's not that buggy as you/we think, in most cases it work without
problem.  What's more important for me is to record the issue and
how to solve it.  So I'd like to have patch 2 and 3 separately.

Thank you for your offer anyway,
--
Shinya Kuribayashi
Renesas Electronics
--
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 v3 1/4] i2c: introduce i2c-cbus driver

2012-11-16 Thread Wolfram Sang
On Mon, Nov 12, 2012 at 09:08:42PM +0200, Aaro Koskinen wrote:
 Add i2c driver to enable access to devices behind CBUS on Nokia Internet
 Tablets.
 
 The patch also adds CBUS I2C configuration for N8x0 which is one of the
 users of this driver.
 
 Cc: linux-i2c@vger.kernel.org
 Acked-by: Felipe Balbi ba...@ti.com
 Acked-by: Tony Lindgren t...@atomide.com
 Signed-off-by: Aaro Koskinen aaro.koski...@iki.fi
 Cc: Wolfram Sang w.s...@pengutronix.de

Mostly good, but the devicetree binding description is missing.
Please add a proper file with the same name as the driver to
Documentation/devicetree/bindings/i2c.
Also, it might make sense to rename the driver to i2c-cbus-gpio?

Thanks,

   Wolfram

-- 
Pengutronix e.K.   | Wolfram Sang|
Industrial Linux Solutions | http://www.pengutronix.de/  |


signature.asc
Description: Digital signature


Re: [PATCH 1/2] i2c-s3c2410: Refactor ifdefs for PM_SLEEP

2012-11-16 Thread Wolfram Sang
On Mon, Nov 05, 2012 at 09:33:38AM +0100, Mark Brown wrote:
 Use the PM_SLEEP ifdef for system suspend and resume. This is partly
 in preparation for adding runtime operations and partly because a user
 may in theory choose to enable runtime suspend but not system suspend.
 
 Signed-off-by: Mark Brown broo...@opensource.wolfsonmicro.com
 Reviewed-by: Shubhrajyoti D shubhrajy...@ti.com

Applied to for-next, thanks!

-- 
Pengutronix e.K.   | Wolfram Sang|
Industrial Linux Solutions | http://www.pengutronix.de/  |


signature.asc
Description: Digital signature


Re: [PATCH 2/2] i2c-s3c2410: Convert to devm_request_and_ioremap()

2012-11-16 Thread Wolfram Sang
On Mon, Nov 05, 2012 at 09:33:39AM +0100, Mark Brown wrote:
 A small code saving and less error handling to worry about.
 
 Signed-off-by: Mark Brown broo...@opensource.wolfsonmicro.com

Applied to for-next, thanks!

-- 
Pengutronix e.K.   | Wolfram Sang|
Industrial Linux Solutions | http://www.pengutronix.de/  |


signature.asc
Description: Digital signature


Re: [PATCH] i2c: s3c2410: Add fix for i2c suspend/resume

2012-11-16 Thread Wolfram Sang
On Wed, Nov 14, 2012 at 09:10:33AM +0530, Abhilash Kesavan wrote:
 Hi,
 
 Any comments on this fix ?

Yes, I agree with Shubhrajyoti here and the code should be refactored so
that gpio initialization is only done once at probe-time unless there is
a real reason to do at suspend/resume.

Thanks,

   Wolfram

-- 
Pengutronix e.K.   | Wolfram Sang|
Industrial Linux Solutions | http://www.pengutronix.de/  |


signature.asc
Description: Digital signature


Re: [PATCH] i2c: i2c-s3c2410: Add support for pinctrl

2012-11-16 Thread Wolfram Sang
On Tue, Nov 13, 2012 at 11:33:40AM +0100, Tomasz Figa wrote:
 This patch adds support for pin configuration using pinctrl subsystem
 to the i2c-s3c2410 driver.
 
 Signed-off-by: Tomasz Figa t.f...@samsung.com
 Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com

Applied to for-next, thanks!

-- 
Pengutronix e.K.   | Wolfram Sang|
Industrial Linux Solutions | http://www.pengutronix.de/  |


signature.asc
Description: Digital signature


Re: [PATCH] i2c: s3c2410: Add fix for i2c suspend/resume

2012-11-16 Thread Wolfram Sang
On Fri, Nov 16, 2012 at 12:46:17PM +0100, Wolfram Sang wrote:
 On Wed, Nov 14, 2012 at 09:10:33AM +0530, Abhilash Kesavan wrote:
  Hi,
  
  Any comments on this fix ?
 
 Yes, I agree with Shubhrajyoti here and the code should be refactored so
 that gpio initialization is only done once at probe-time unless there is
 a real reason to do at suspend/resume.

Note that I just applied a few patches to s3c2410 also touching this
area. Would be nice if you could wait a bit until I pushed out my
for-next branch later today and base on that. Or wait for 3.8-rc1.

Thanks,

   Wolfram

-- 
Pengutronix e.K.   | Wolfram Sang|
Industrial Linux Solutions | http://www.pengutronix.de/  |


signature.asc
Description: Digital signature


Re: [PATCH v3 3/5] i2c: i2c-mpc: make I2C bus speed configurable

2012-11-16 Thread Wolfram Sang
On Wed, Nov 07, 2012 at 10:40:47AM -0600, Timur Tabi wrote:
 Wolfgang,
 
 I know it's been 3 1/2 years since you wrote this code, but I think I
 found a bug.
 
 On Tue, Apr 7, 2009 at 3:20 AM, Wolfgang Grandegger w...@grandegger.com 
 wrote:
  This patch makes the I2C bus speed configurable by using the I2C node
  property clock-frequency. If the property is not defined, the old
  fixed clock settings will be used for backward comptibility.
 
  The generic I2C clock properties, especially the CPU-specific source
  clock pre-scaler are defined via the OF match table:
 
static const struct of_device_id mpc_i2c_of_match[] = {
  ...
  {.compatible = fsl,mpc8543-i2c,
   .data = (struct fsl_i2c_match_data) {
  .setclock = mpc_i2c_setclock_8xxx,
  .prescaler = 2,
  },
  },
 
  The data field defines the relevant I2C setclock function and the
  relevant pre-scaler for the I2C source clock frequency.
 
  It uses arch-specific tables and functions to determine resonable
  Freqency Divider Register (fdr) values for MPC83xx, MPC85xx, MPC86xx,
  MPC5200 and MPC5200B.
 
  The i2c-flags field and the corresponding FSL_I2C_DEV_* definitions
  have been removed as they are obsolete.
 
  Signed-off-by: Wolfgang Grandegger w...@grandegger.com
 
 ...
 
  +u32 mpc_i2c_get_sec_cfg_8xxx(void)
  +{
  +   struct device_node *node = NULL;
  +   u32 __iomem *reg;
  +   u32 val = 0;
  +
  +   node = of_find_node_by_name(NULL, global-utilities);
  +   if (node) {
  +   const u32 *prop = of_get_property(node, reg, NULL);
  +   if (prop) {
  +   /*
  +* Map and check POR Device Status Register 2
  +* (PORDEVSR2) at 0xE0014
  +*/
  +   reg = ioremap(get_immrbase() + *prop + 0x14, 0x4);
  +   if (!reg)
  +   printk(KERN_ERR
  +  Error: couldn't map PORDEVSR2\n);
  +   else
  +   val = in_be32(reg)  0x0080; /* sec-cfg 
  */
 
 I'm looking at the MPC8544 reference manual, and PORDEVSR2[SEC_CFG] is
 in position 26, which means that this line should be  0x20, not 
 0x80.
 
 Can you check this for me and let me know if I'm right?

Ping. Somebody wants to send a patch?

-- 
Pengutronix e.K.   | Wolfram Sang|
Industrial Linux Solutions | http://www.pengutronix.de/  |


signature.asc
Description: Digital signature


Re: [PATCH 11/11] i2c: pxa: no need slave addr for i2c master mode reset

2012-11-16 Thread Wolfram Sang
Hi,

On Thu, Nov 08, 2012 at 06:25:24AM -0800, Leo Song wrote:

 Would you please help to review and merge these 11 patches for
 drivers/i2c/busses/i2c-pxa.c?

There are already a few formal things:

- For large series, please write a cover-letter describing the series in
  total (use --cover-letter). The omap guys do this very well, check the
  list archive.

- why would upstream need Change-Id: in the patch description?

- the cover letter should also state what testing you did. the pxa
  driver is used in lots of different SoCs and I'd like to know how
  much this was tested on other systems regarding regressions.

- if you can't test it on other machines, ask for help. Putting alkml on
  CC is one way to do this

- I'd ask you to add alkml anyway since your patches include hooks to
  the mach and there are people having more experience on this platform
  than me

I didn't really have a look at the code yet, since I ask you to resend
it fixing the above points first. But from a glimpse, you should try to
avoid adding function pointers to platform_data at all costs. Maybe you
can think of alternative solutions.

Regards and thanks for the submission,

   Wolfram

-- 
Pengutronix e.K.   | Wolfram Sang|
Industrial Linux Solutions | http://www.pengutronix.de/  |


signature.asc
Description: Digital signature


Re: [PATCH] OMAP: board-files: fix i2c_bus for tfp410

2012-11-16 Thread Felipe Balbi
Hi,

On Fri, Nov 16, 2012 at 02:22:33PM +0200, Tomi Valkeinen wrote:
 The i2c handling in tfp410 driver, which handles converting parallel RGB
 to DVI, was changed in 958f2717b84e88bf833d996997fda8f73276f2af. The

commit summary should be added in () after commit hash. This would look
like:

'was changed in 958f271 (OMAPDSS: TFP410: pdata rewrite).'

 patch changed what value the driver considers as invalid/undefined.
 Before the patch 0 was the invalid value, but as 0 is a valid bus
  ^
  missing comma (,) character here.

 number, the patch changed this to -1.
 
 However, the fact was missed that many board files do not define the bus
 number at all, thus it's left to 0. This causes the driver to fail to
 get the i2c bus, exiting from the driver's probe with an error, meaning
 that the DVI output does not work for those boards.
 
 This patch fixes the issue by changing the i2c_bus number field in the
 driver's platform data from u16 to int, and setting the bus number to -1
 in the board files for the boards that did not define the bus. The
 exception is devkit8000, for which the bus is set to 1, which is the
 correct bus for that board.
 
 The bug exists in v3.5+ kernels.
 
 Signed-off-by: Tomi Valkeinen tomi.valkei...@ti.com
 Reported-by: Thomas Weber tho...@tomweber.eu
 [for v3.5, v3.6 stable kernels]
 Cc: sta...@vger.kernel.org

This format is peculiar. Usually people use:

Cc: sta...@vger.kernel.org # v3.5 v3.6

To be fair, the whole i2c_bus_num looks like a big hackery introduced by
the way panel drivers are written for OMAP DSS.

TFP410 is an I2C client, not an OMAPDSS client. After a quick look at
the driver, there's is no such thing as a DSS bus, so looks like you
should have an I2C driver for TFP410 and the whole DSS stuff should be
just a list of clients, but not a struct bus at all.

The fact that you have to pass the I2C bus number down to the panel
driver is already a big indication of how wrong this is, IMHO.

-- 
balbi


signature.asc
Description: Digital signature


Re: [PATCH 2/2] ARM: dts: cfa10049: Add the i2c muxer buses to the CFA-10049

2012-11-16 Thread Wolfram Sang
On Fri, Nov 16, 2012 at 10:25:32PM +0800, Shawn Guo wrote:
 On Fri, Nov 16, 2012 at 09:32:18AM +0100, Wolfram Sang wrote:
  On Thu, Oct 25, 2012 at 06:23:54PM +0200, Maxime Ripard wrote:
   This will allow to add the 3 Nuvoton NAU7802 ADCs and the NXP PCA9555
   GPIO expander eventually.
   
   Signed-off-by: Maxime Ripard maxime.rip...@free-electrons.com
  
  Shawn, I pushed the needed code to my for-next now. So you could apply
  this patch for 3.8 or give an ack in case you want me to pick it via
  i2c.
  
 I have already sent mxs 3.8 stuff to arm-soc, so please have it go via
 i2c tree.  Thanks.
 
 Acked-by: Shawn Guo shawn@linaro.org

OK. Applied to my for-next, thanks!

-- 
Pengutronix e.K.   | Wolfram Sang|
Industrial Linux Solutions | http://www.pengutronix.de/  |


signature.asc
Description: Digital signature


Re: [PATCH 2/2] ARM: dts: cfa10049: Add the i2c muxer buses to the CFA-10049

2012-11-16 Thread Shawn Guo
On Fri, Nov 16, 2012 at 09:32:18AM +0100, Wolfram Sang wrote:
 On Thu, Oct 25, 2012 at 06:23:54PM +0200, Maxime Ripard wrote:
  This will allow to add the 3 Nuvoton NAU7802 ADCs and the NXP PCA9555
  GPIO expander eventually.
  
  Signed-off-by: Maxime Ripard maxime.rip...@free-electrons.com
 
 Shawn, I pushed the needed code to my for-next now. So you could apply
 this patch for 3.8 or give an ack in case you want me to pick it via
 i2c.
 
I have already sent mxs 3.8 stuff to arm-soc, so please have it go via
i2c tree.  Thanks.

Acked-by: Shawn Guo shawn@linaro.org

--
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: mxs: Handle i2c DMA failure properly

2012-11-16 Thread Wolfram Sang
Tim,

On Wed, Nov 14, 2012 at 09:34:27PM -0600, Tim Michals wrote:
This patch
�@@ -150,8 +150,6 @@ static void mxs_i2c_reset(struct mxs_i2c
�   writel(i2c-speed-timing2, i2c-regs + MXS_I2C_TIMING2);
�
�   writel(MXS_I2C_IRQ_MASK  8, i2c-regs + MXS_I2C_CTRL1_SET);
-
-   dmaengine_terminate_all(i2c-dmach);
�}��
This patch fixes current DMA requests which time out or error returned by
the ISR.�
Tim

Please elaborate because I am confused. Did you see problems after
applying Marek's patch and if so, please describe in what circumstances
so we could reproduce.

Thanks,

   Wolfram

-- 
Pengutronix e.K.   | Wolfram Sang|
Industrial Linux Solutions | http://www.pengutronix.de/  |


signature.asc
Description: Digital signature


Re: [PATCHv3] i2c: omap: Move the remove constraint

2012-11-16 Thread Wolfram Sang
On Thu, Nov 15, 2012 at 02:19:21PM +0530, Shubhrajyoti D wrote:
 Currently we just queue the transfer and release the
 qos constraints, however we do not wait for the transfer
 to complete to release the constraint. Move the remove
 constraint after the bus busy as we are sure that the
 transfers are completed by then.
 
 Acked-by: Jean Pihet j-pi...@ti.com
 Signed-off-by: Shubhrajyoti D shubhrajy...@ti.com

Applied to for-next. Please let me know if it should go to for-current.

-- 
Pengutronix e.K.   | Wolfram Sang|
Industrial Linux Solutions | http://www.pengutronix.de/  |


signature.asc
Description: Digital signature


Re: [PATCH] OMAP: board-files: fix i2c_bus for tfp410

2012-11-16 Thread Tomi Valkeinen
On 2012-11-16 15:51, Felipe Balbi wrote:
 Hi,
 
 On Fri, Nov 16, 2012 at 02:22:33PM +0200, Tomi Valkeinen wrote:
 The i2c handling in tfp410 driver, which handles converting parallel RGB
 to DVI, was changed in 958f2717b84e88bf833d996997fda8f73276f2af. The
 
 commit summary should be added in () after commit hash. This would look
 like:
 
 'was changed in 958f271 (OMAPDSS: TFP410: pdata rewrite).'

Yep.

 patch changed what value the driver considers as invalid/undefined.
 Before the patch 0 was the invalid value, but as 0 is a valid bus
   ^
 missing comma (,) character here.

Right.

 number, the patch changed this to -1.

 However, the fact was missed that many board files do not define the bus
 number at all, thus it's left to 0. This causes the driver to fail to
 get the i2c bus, exiting from the driver's probe with an error, meaning
 that the DVI output does not work for those boards.

 This patch fixes the issue by changing the i2c_bus number field in the
 driver's platform data from u16 to int, and setting the bus number to -1
 in the board files for the boards that did not define the bus. The
 exception is devkit8000, for which the bus is set to 1, which is the
 correct bus for that board.

 The bug exists in v3.5+ kernels.

 Signed-off-by: Tomi Valkeinen tomi.valkei...@ti.com
 Reported-by: Thomas Weber tho...@tomweber.eu
 [for v3.5, v3.6 stable kernels]
 Cc: sta...@vger.kernel.org
 
 This format is peculiar. Usually people use:
 
 Cc: sta...@vger.kernel.org # v3.5 v3.6

Yes, I tried that. But my git send-email (1.7.10.4) rejects that line. I
don't know if it's my setup, that particular git version, or what...

 To be fair, the whole i2c_bus_num looks like a big hackery introduced by
 the way panel drivers are written for OMAP DSS.
 
 TFP410 is an I2C client, not an OMAPDSS client. After a quick look at
 the driver, there's is no such thing as a DSS bus, so looks like you
 should have an I2C driver for TFP410 and the whole DSS stuff should be
 just a list of clients, but not a struct bus at all.
 
 The fact that you have to pass the I2C bus number down to the panel
 driver is already a big indication of how wrong this is, IMHO.

Without going deeper in the dss device model problems, I would agree
with you if this was about i2c panel, but this is not quite like that.

A panel controlled via i2c would be an i2c device. But TFP410 is not
controlled via i2c. It's not really controlled at all except via
power-down gpio. TFP410 doesn't need the i2c to be functional at all.

The i2c lines do not even touch TFP410 chip, so to be precise, the i2c
lines should not be TFP410's concern. The i2c lines come from the
monitor and go to OMAP's i2c pins. But TFP410 driver is a convenient
place to manage them.

(As a side note, TFP410 _could_ be controlled via i2c, if it would've
been setup so in the board hardware. That would be totally different
thing, though, than what's discussed here.).

Anyway, this patch wasn't meant to fix dss device model problems. It's
meant to fix a bug in the kernel.

 Tomi




signature.asc
Description: OpenPGP digital signature


Re: [PATCH v3] i2c-hid: introduce HID over i2c specification implementation

2012-11-16 Thread Benjamin Tissoires
On Thu, Nov 15, 2012 at 3:04 PM, Benjamin Tissoires
benjamin.tissoi...@gmail.com wrote:
 On Thu, Nov 15, 2012 at 2:51 PM, Jiri Kosina jkos...@suse.cz wrote:
 On Mon, 12 Nov 2012, Benjamin Tissoires wrote:

 Microsoft published the protocol specification of HID over i2c:
 http://msdn.microsoft.com/en-us/library/windows/hardware/hh852380.aspx

 This patch introduces an implementation of this protocol.

 This implementation does not includes the ACPI part of the specification.
 This will come when ACPI 5.0 devices enumeration will be available.

 Once the ACPI part is done, OEM will not have to declare HID over I2C
 devices in their platform specific driver.

 Signed-off-by: Benjamin Tissoires benjamin.tissoi...@gmail.com

 Out of curiosity -- has this been tested on a real device (is there any
 such device available anyway?), or is that just the implementation of the
 defined protocol?

 It has been tested on an ELAN microelectronics device (a prototype),
 on an odroid-x board. That's how we figure out the bug in the
 set_report command.
 I think we manage to test all main features of the protocol
 (get_report, irqs, hid descriptor, report descriptors, set_report).

 I'm currently waiting for a Synaptics touchpad to check if it's also
 working with their devices.

 The thing is that HID over i2c for x86 platform will presumably
 require the Haswell platform from Intel (we need ACPI 5 for
 enumeration), but it would be very nice to get this in the kernel just
 before hardware arrive on the market :)
 However, I won't be surprise if android OEMs also start using this
 specification because it won't force them to write kernel drivers...

And as a complement, Ramalingam tested it for Nvidia on an early
NVIDIA's Tegra reference board for PISMO which is registered at
http://www.arm.linux.org.uk/developer/machines/list.php?id=4439 , with
a HID over i2c keyboard.
He is up to test also his Synaptics touchpad.

Cheers,
Benjamin


 Cheers,
 Benjamin


 Thanks,

 --
 Jiri Kosina
 SUSE Labs
--
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 0/2] i2c: i2c-ocores: Add support for sparc, custom set and get functions, and the GRLIB port of the controller

2012-11-16 Thread Wolfram Sang
On Thu, Nov 15, 2012 at 04:50:57PM +0100, Andreas Larsson wrote:
 On sparc, irqs are not present as an IORESOURCE in the struct platform_device
 representation. By using platform_get_irq instead of platform_get_resource the
 driver works for sparc.
 
 The GRLIB port of the ocores i2c controller needs custom getreg and setreg
 functions to allow for big endian register access and to deal with the fact 
 that
 the PRELOW and PREHIGH registers have been merged into one register.
 
 Signed-off-by: Andreas Larsson andr...@gaisler.com

Thanks, applied to for-next. I like the outcome, so thanks to Peter as
well.

-- 
Pengutronix e.K.   | Wolfram Sang|
Industrial Linux Solutions | http://www.pengutronix.de/  |


signature.asc
Description: Digital signature


Re: [PATCH] OMAP: board-files: fix i2c_bus for tfp410

2012-11-16 Thread Felipe Balbi
Hi,

On Fri, Nov 16, 2012 at 04:27:01PM +0200, Tomi Valkeinen wrote:
 On 2012-11-16 15:51, Felipe Balbi wrote:
  Hi,
  
  On Fri, Nov 16, 2012 at 02:22:33PM +0200, Tomi Valkeinen wrote:
  The i2c handling in tfp410 driver, which handles converting parallel RGB
  to DVI, was changed in 958f2717b84e88bf833d996997fda8f73276f2af. The
  
  commit summary should be added in () after commit hash. This would look
  like:
  
  'was changed in 958f271 (OMAPDSS: TFP410: pdata rewrite).'
 
 Yep.
 
  patch changed what value the driver considers as invalid/undefined.
  Before the patch 0 was the invalid value, but as 0 is a valid bus
^
missing comma (,) character here.
 
 Right.
 
  number, the patch changed this to -1.
 
  However, the fact was missed that many board files do not define the bus
  number at all, thus it's left to 0. This causes the driver to fail to
  get the i2c bus, exiting from the driver's probe with an error, meaning
  that the DVI output does not work for those boards.
 
  This patch fixes the issue by changing the i2c_bus number field in the
  driver's platform data from u16 to int, and setting the bus number to -1
  in the board files for the boards that did not define the bus. The
  exception is devkit8000, for which the bus is set to 1, which is the
  correct bus for that board.
 
  The bug exists in v3.5+ kernels.
 
  Signed-off-by: Tomi Valkeinen tomi.valkei...@ti.com
  Reported-by: Thomas Weber tho...@tomweber.eu
  [for v3.5, v3.6 stable kernels]
  Cc: sta...@vger.kernel.org
  
  This format is peculiar. Usually people use:
  
  Cc: sta...@vger.kernel.org # v3.5 v3.6
 
 Yes, I tried that. But my git send-email (1.7.10.4) rejects that line. I
 don't know if it's my setup, that particular git version, or what...

weird... I never had that problem, since git 1.6.x, I have never seen
that and I tend to upgrade rather frequently. I'm using 1.8.0 now and
have sent a few patches to stable recently with no problems.

  To be fair, the whole i2c_bus_num looks like a big hackery introduced by
  the way panel drivers are written for OMAP DSS.
  
  TFP410 is an I2C client, not an OMAPDSS client. After a quick look at
  the driver, there's is no such thing as a DSS bus, so looks like you
  should have an I2C driver for TFP410 and the whole DSS stuff should be
  just a list of clients, but not a struct bus at all.
  
  The fact that you have to pass the I2C bus number down to the panel
  driver is already a big indication of how wrong this is, IMHO.
 
 Without going deeper in the dss device model problems, I would agree
 with you if this was about i2c panel, but this is not quite like that.
 
 A panel controlled via i2c would be an i2c device. But TFP410 is not
 controlled via i2c. It's not really controlled at all except via
 power-down gpio. TFP410 doesn't need the i2c to be functional at all.

then why does it need the i2c adapter ? What is this power-down gpio ?
Should that be hidden under gpiolib instead ?

 The i2c lines do not even touch TFP410 chip, so to be precise, the i2c
 lines should not be TFP410's concern. The i2c lines come from the
 monitor and go to OMAP's i2c pins. But TFP410 driver is a convenient
 place to manage them.

fair enough... but who's actually using those i2c lines ? OMAP is the
I2C master, who's the slave ? It's something in the monitor, I assume...

IIUC, this I2C bus goes over the HDMI wire ?

 (As a side note, TFP410 _could_ be controlled via i2c, if it would've
 been setup so in the board hardware. That would be totally different
 thing, though, than what's discussed here.).
 
 Anyway, this patch wasn't meant to fix dss device model problems. It's
 meant to fix a bug in the kernel.

I understand... should've added that this part wasn't related to
$SUBJECT.

-- 
balbi


signature.asc
Description: Digital signature


Re: [PATCH] i2c: mxs: Handle i2c DMA failure properly

2012-11-16 Thread Fabio Estevam
Hi Tim,

On Fri, Nov 16, 2012 at 1:24 PM, Tim Michals tcmich...@gmail.com wrote:

 My suggestion was to always clear the DMA in the reset i2c code.  This way
 if there is an error reported from the ISR or a timeout, it always clears
 it.  I've tested this approach with I2C devices connected and and doing a
 i2cdetect -r 0.

Thanks for the explanation.

Maybe you could post your proposed patch?

Thanks,

Fabio Estevam
--
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] OMAP: board-files: fix i2c_bus for tfp410

2012-11-16 Thread Tomi Valkeinen
On 2012-11-16 17:19, Felipe Balbi wrote:
 Hi,
 
 On Fri, Nov 16, 2012 at 04:27:01PM +0200, Tomi Valkeinen wrote:
 On 2012-11-16 15:51, Felipe Balbi wrote:
 Hi,

 On Fri, Nov 16, 2012 at 02:22:33PM +0200, Tomi Valkeinen wrote:
 The i2c handling in tfp410 driver, which handles converting parallel RGB
 to DVI, was changed in 958f2717b84e88bf833d996997fda8f73276f2af. The

 commit summary should be added in () after commit hash. This would look
 like:

 'was changed in 958f271 (OMAPDSS: TFP410: pdata rewrite).'

 Yep.

 patch changed what value the driver considers as invalid/undefined.
 Before the patch 0 was the invalid value, but as 0 is a valid bus
   ^
   missing comma (,) character here.

 Right.

 number, the patch changed this to -1.

 However, the fact was missed that many board files do not define the bus
 number at all, thus it's left to 0. This causes the driver to fail to
 get the i2c bus, exiting from the driver's probe with an error, meaning
 that the DVI output does not work for those boards.

 This patch fixes the issue by changing the i2c_bus number field in the
 driver's platform data from u16 to int, and setting the bus number to -1
 in the board files for the boards that did not define the bus. The
 exception is devkit8000, for which the bus is set to 1, which is the
 correct bus for that board.

 The bug exists in v3.5+ kernels.

 Signed-off-by: Tomi Valkeinen tomi.valkei...@ti.com
 Reported-by: Thomas Weber tho...@tomweber.eu
 [for v3.5, v3.6 stable kernels]
 Cc: sta...@vger.kernel.org

 This format is peculiar. Usually people use:

 Cc: sta...@vger.kernel.org # v3.5 v3.6

 Yes, I tried that. But my git send-email (1.7.10.4) rejects that line. I
 don't know if it's my setup, that particular git version, or what...
 
 weird... I never had that problem, since git 1.6.x, I have never seen
 that and I tend to upgrade rather frequently. I'm using 1.8.0 now and
 have sent a few patches to stable recently with no problems.
 
 To be fair, the whole i2c_bus_num looks like a big hackery introduced by
 the way panel drivers are written for OMAP DSS.

 TFP410 is an I2C client, not an OMAPDSS client. After a quick look at
 the driver, there's is no such thing as a DSS bus, so looks like you
 should have an I2C driver for TFP410 and the whole DSS stuff should be
 just a list of clients, but not a struct bus at all.

 The fact that you have to pass the I2C bus number down to the panel
 driver is already a big indication of how wrong this is, IMHO.

 Without going deeper in the dss device model problems, I would agree
 with you if this was about i2c panel, but this is not quite like that.

 A panel controlled via i2c would be an i2c device. But TFP410 is not
 controlled via i2c. It's not really controlled at all except via
 power-down gpio. TFP410 doesn't need the i2c to be functional at all.
 
 then why does it need the i2c adapter ? What is this power-down gpio ?
 Should that be hidden under gpiolib instead ?

For the i2c, see below. Power-down GPIO is used to power down and up the
tfp410 chip.

 The i2c lines do not even touch TFP410 chip, so to be precise, the i2c
 lines should not be TFP410's concern. The i2c lines come from the
 monitor and go to OMAP's i2c pins. But TFP410 driver is a convenient
 place to manage them.
 
 fair enough... but who's actually using those i2c lines ? OMAP is the
 I2C master, who's the slave ? It's something in the monitor, I assume...
 
 IIUC, this I2C bus goes over the HDMI wire ?

Yes, the i2c goes over HDMI wire. OMAP is the master, monitor is the
slave. You can see some more info from
http://en.wikipedia.org/wiki/Display_Data_Channel under DDC2 section.

It is used to read the EDID
(http://en.wikipedia.org/wiki/Extended_display_identification_data)
information from the monitor, which tells things like supported video
timings etc.

As for why the tfp410 driver handles the i2c... We don't have a better
place. There's no driver for the monitor. Although in the future with
common panel framework perhaps we will.

 Tomi




signature.asc
Description: OpenPGP digital signature


Re: [PATCH v3 3/5] i2c: i2c-mpc: make I2C bus speed configurable

2012-11-16 Thread Timur Tabi
Wolfram Sang wrote:
 Ping. Somebody wants to send a patch?

I already have a patch that fixes it, I just want confirmation that I'm
right first.

-- 
Timur Tabi
Linux kernel developer at Freescale

--
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 0/3] i2c: at91: add dma support

2012-11-16 Thread Wolfram Sang
Hi Ludovic,

 Here is the third version taking into account your feedback.

Thanks, looks mostly good. I have problems applying patch 3. What is
this series based against?

 Patch 1/3 should go into v3.7.

No, not necessarily. It is not really a bugfix, and it is probably too
late in the cycle for compiler silencing. Ah well, I decide later.

BTW if you use --strict with checkpatch, it will tell you about the
space after casts :)

Regards,

   Wolfram

-- 
Pengutronix e.K.   | Wolfram Sang|
Industrial Linux Solutions | http://www.pengutronix.de/  |


signature.asc
Description: Digital signature


tfp410 and i2c_bus_num (was: Re: [PATCH] OMAP: board-files: fix i2c_bus for tfp410)

2012-11-16 Thread Felipe Balbi
Hi,

On Fri, Nov 16, 2012 at 05:39:44PM +0200, Tomi Valkeinen wrote:
  To be fair, the whole i2c_bus_num looks like a big hackery introduced by
  the way panel drivers are written for OMAP DSS.
 
  TFP410 is an I2C client, not an OMAPDSS client. After a quick look at
  the driver, there's is no such thing as a DSS bus, so looks like you
  should have an I2C driver for TFP410 and the whole DSS stuff should be
  just a list of clients, but not a struct bus at all.
 
  The fact that you have to pass the I2C bus number down to the panel
  driver is already a big indication of how wrong this is, IMHO.
 
  Without going deeper in the dss device model problems, I would agree
  with you if this was about i2c panel, but this is not quite like that.
 
  A panel controlled via i2c would be an i2c device. But TFP410 is not
  controlled via i2c. It's not really controlled at all except via
  power-down gpio. TFP410 doesn't need the i2c to be functional at all.
  
  then why does it need the i2c adapter ? What is this power-down gpio ?
  Should that be hidden under gpiolib instead ?
 
 For the i2c, see below. Power-down GPIO is used to power down and up the
 tfp410 chip.

that much I guessed ;-) Should it be hidden under gpiolib ?

  The i2c lines do not even touch TFP410 chip, so to be precise, the i2c
  lines should not be TFP410's concern. The i2c lines come from the
  monitor and go to OMAP's i2c pins. But TFP410 driver is a convenient
  place to manage them.
  
  fair enough... but who's actually using those i2c lines ? OMAP is the
  I2C master, who's the slave ? It's something in the monitor, I assume...
  
  IIUC, this I2C bus goes over the HDMI wire ?
 
 Yes, the i2c goes over HDMI wire. OMAP is the master, monitor is the
 slave. You can see some more info from
 http://en.wikipedia.org/wiki/Display_Data_Channel under DDC2 section.
 
 It is used to read the EDID
 (http://en.wikipedia.org/wiki/Extended_display_identification_data)
 information from the monitor, which tells things like supported video
 timings etc.
 
 As for why the tfp410 driver handles the i2c... We don't have a better
 place. There's no driver for the monitor. Although in the future with

than that's wrong :-) If TFP410 isn't really using I2C it shouldn't need
the whole i2c_bus_num logic. I'm far from fully understanding dss
architecture but it looks like what you want is a generic 'i2c-edid.c'
which just reads the edid structure during probe and caches the values
and exposes them via sysfs ?!? (perhaps you also need a kernel API to
read those values... I don't know; but that's also doable).

If you have a generic i2c-edid.c simple driver, I guess X could be
changes to read those values from sysfs and take actions based on those.

Looks like even drm_edid.c should change, btw.

 common panel framework perhaps we will.

ok, good ;-)

-- 
balbi


signature.asc
Description: Digital signature


Re: tfp410 and i2c_bus_num

2012-11-16 Thread Tomi Valkeinen
On 2012-11-16 20:21, Felipe Balbi wrote:
 Hi,
 
 On Fri, Nov 16, 2012 at 05:39:44PM +0200, Tomi Valkeinen wrote:
 To be fair, the whole i2c_bus_num looks like a big hackery introduced by
 the way panel drivers are written for OMAP DSS.

 TFP410 is an I2C client, not an OMAPDSS client. After a quick look at
 the driver, there's is no such thing as a DSS bus, so looks like you
 should have an I2C driver for TFP410 and the whole DSS stuff should be
 just a list of clients, but not a struct bus at all.

 The fact that you have to pass the I2C bus number down to the panel
 driver is already a big indication of how wrong this is, IMHO.

 Without going deeper in the dss device model problems, I would agree
 with you if this was about i2c panel, but this is not quite like that.

 A panel controlled via i2c would be an i2c device. But TFP410 is not
 controlled via i2c. It's not really controlled at all except via
 power-down gpio. TFP410 doesn't need the i2c to be functional at all.

 then why does it need the i2c adapter ? What is this power-down gpio ?
 Should that be hidden under gpiolib instead ?

 For the i2c, see below. Power-down GPIO is used to power down and up the
 tfp410 chip.
 
 that much I guessed ;-) Should it be hidden under gpiolib ?

I have to say I have no idea what you mean... Hidden how?

 The i2c lines do not even touch TFP410 chip, so to be precise, the i2c
 lines should not be TFP410's concern. The i2c lines come from the
 monitor and go to OMAP's i2c pins. But TFP410 driver is a convenient
 place to manage them.

 fair enough... but who's actually using those i2c lines ? OMAP is the
 I2C master, who's the slave ? It's something in the monitor, I assume...

 IIUC, this I2C bus goes over the HDMI wire ?

 Yes, the i2c goes over HDMI wire. OMAP is the master, monitor is the
 slave. You can see some more info from
 http://en.wikipedia.org/wiki/Display_Data_Channel under DDC2 section.

 It is used to read the EDID
 (http://en.wikipedia.org/wiki/Extended_display_identification_data)
 information from the monitor, which tells things like supported video
 timings etc.

 As for why the tfp410 driver handles the i2c... We don't have a better
 place. There's no driver for the monitor. Although in the future with
 
 than that's wrong :-) If TFP410 isn't really using I2C it shouldn't need
 the whole i2c_bus_num logic. I'm far from fully understanding dss
 architecture but it looks like what you want is a generic 'i2c-edid.c'
 which just reads the edid structure during probe and caches the values
 and exposes them via sysfs ?!? (perhaps you also need a kernel API to
 read those values... I don't know; but that's also doable).

Well, it's not that simple. TFP410 manages hot plug detection of the
HDMI cable (although not implemented currently), so the we'd need to
convey that information to the i2c-edid somehow. Which, of course, is
doable, but increases complexity.

Another thing is that even if in this case the i2c and EDID reading are
separate from the actual video path, that may not be the case for other
display solutions. We could have a DSI panel which reports its
resolution via DSI bus, or we could have an external DSI-to-HDMI chip,
which reads the EDID via i2c from the monitor, but reports them back to
the SoC via DSI bus.

So we need a generic way to get the resolution information, and it makes
sense to have this in the components of the video path, not in a
separate driver which is not part of the video path as such.

And while the above issues could perhaps be solved, all we do is read
one or two 128 byte datablocks from the i2c device. I'm not sure if the
extra complexity is worth it. At least it's not on the top of my todo
list as long as the current solution works =).

 If you have a generic i2c-edid.c simple driver, I guess X could be
 changes to read those values from sysfs and take actions based on those.

We handle the EDID inside the kernel, although I'm not sure if drm also
exposes the EDID to userspace.

 Looks like even drm_edid.c should change, btw.

Yes, DRM handles it in somewhat similar way.

 Tomi




signature.asc
Description: OpenPGP digital signature