Re: [PATCH] i2c-piix4 - Add support for secondary SMBus on AMD SB800 and AMD FCH chipsets

2013-06-11 Thread Rudolf Marek

Do you know if there should be something connected?


In Paul case, most likely not. In my case there is a voltage controller for the 
DDR3 voltage regulator. I asked Paul test it on his SB800 system, I have the 
Hudson successor..



Rudolf, it looks like your patch could also update the i2c files under
`Documentation`. At least there are some patches doing that.


Do you mean 'busses/i2c-piix4'?


Yes he means that. I will add documentation to the patch.

Thanks
Rudolf

--
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: use platform_{get,set}_drvdata()

2013-06-11 Thread Wolfram Sang
On Thu, May 23, 2013 at 07:22:40PM +0900, Jingoo Han wrote:
> Use the wrapper functions for getting and setting the driver data using
> platform_device instead of using dev_{get,set}_drvdata() with &pdev->dev,
> so we can directly pass a struct platform_device.
> 
> Signed-off-by: Jingoo Han 

Applied to for-next, thanks!




signature.asc
Description: Digital signature


Re: [PATCH] i2c-piix4 - Add support for secondary SMBus on AMD SB800 and AMD FCH chipsets

2013-06-11 Thread Wolfram Sang
> > To detect i2c devices on the second bus.
> 
> $ sudo i2cdetect 9
> WARNING! This program can confuse your I2C bus, cause data loss and 
> worse!
> I will probe file /dev/i2c-9.
> I will probe address range 0x03-0x77.
> Continue? [Y/n] 
>  0  1  2  3  4  5  6  7  8  9  a  b  c  d  e  f
> 00:  -- -- -- -- -- -- -- -- -- -- -- -- -- 
> 10: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 
> 20: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 
> 30: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 
> 40: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 
> 50: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 
> 60: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 
> 70: -- -- -- -- -- -- -- --

Do you know if there should be something connected?

> Rudolf, it looks like your patch could also update the i2c files under
> `Documentation`. At least there are some patches doing that.

Do you mean 'busses/i2c-piix4'?



signature.asc
Description: Digital signature


Re: [PATCH] i2c: i2c-mpc: make mpc_i2c_pm_ops static

2013-06-11 Thread Wolfram Sang
On Tue, Mar 19, 2013 at 10:07:28AM +0900, Jingoo Han wrote:
> mpc_i2c_pm_ops is not exported. Also, CONFIG_PM_SLEEP is used to
> remove unnecessary ifdefs.

I guess we should use the new pm_ops_ptr macro which is scheduled for
3.11.

Thanks,

   Wolfram



signature.asc
Description: Digital signature


Re: [PATCH] i2c-designware-platform: don't check resource with devm_ioremap_resource

2013-06-11 Thread Wolfram Sang
On Fri, Jun 07, 2013 at 04:57:29PM +0300, Andy Shevchenko wrote:
> devm_ioremap_resource does sanity checks on the given resource. No need to
> duplicate this in the driver.
> 
> Signed-off-by: Andy Shevchenko 

Thanks, yet I applied my original patch which removes this pattern from
the whole subsystem.



signature.asc
Description: Digital signature


Re: [RFC PATCH] i2c-designware-core: disable adapter before fill dev structure

2013-06-11 Thread Wolfram Sang
On Fri, Jun 07, 2013 at 03:01:34PM +0200, Christian Ruppert wrote:
> Hi Andy,
> 
> I like your patch and I just did some testing on it. Unluckily, it
> doesn't solve the lock-up problem.
> 
> I haven't investigated any further but I suspect that on top of the
> cases I observed when debugging this (interrupts after initialisation of
> dev, easy to prove) there are more obscure cases in which interrupts are
> generated in an unexpected manner after errors. The interrupt-driven
> transfer state machine of the driver implicitly relies on the fact that
> all updates of dev which are related to the same transfer are performed
> between the mutex_lock and mutex_unlock calls in i2c_dw_xfer. Thus, I
> decided it was safer to disable the block before releasing the mutex
> when I wrote my patch.
> 
> That said, I think the sequencing at transfer initialisation is more
> logical with your patch and I wonder if it is still worth applying. Any
> other opinions on this?

Ping. There are:

[V2] i2c: designware: fix race between subsequent xfers
[RFC] i2c-designware-core: disable adapter before fill dev structure

What is the consensus of those two patches?



signature.asc
Description: Digital signature


Re: [PATCH] i2c: designware: prevent signals from aborting I2C transfers

2013-06-11 Thread Wolfram Sang
On Wed, May 22, 2013 at 01:03:11PM +0300, Mika Westerberg wrote:
> If a process receives signal while it is waiting for I2C transfer to
> complete, an error is returned to the caller and the transfer is aborted.
> This can cause the driver to fail subsequent transfers. Also according to
> commit d295a86eab2 (i2c: mv64xxx: work around signals causing I2C
> transactions to be aborted) I2C drivers aren't supposed to abort
> transactions on signals.
> 
> To prevent this switch to use wait_for_completion_timeout() instead of
> wait_for_completion_interruptible_timeout() in the designware I2C driver.
> 
> Signed-off-by: Mika Westerberg 

Applied to for-next, thanks!



signature.asc
Description: Digital signature


Re: [PATCHv3 1/6] i2c: sunxi: Add Allwinner A1X i2c driver

2013-06-11 Thread Wolfram Sang
On Mon, Jun 10, 2013 at 12:08:47PM +0200, Maxime Ripard wrote:
> Hi Wolfram,
> 
> On Wed, Jun 05, 2013 at 11:31:44PM +0200, Maxime Ripard wrote:
> > I guess it would make the code much more complicated for such simple
> > drivers, so I wouldn't push too much for merging, but I guess it's your
> > call.
> 
> I'd like this to be in 3.11, would merging the driver separately for
> this release and merging it with mv64xxx for 3.12 be acceptable to you ?

Thanks for the offer, but the chance of a regression is too high when
switching drivers. Also, configs would need updates then, etc...



signature.asc
Description: Digital signature


Re: [PATCHv3 1/6] i2c: sunxi: Add Allwinner A1X i2c driver

2013-06-11 Thread Wolfram Sang
On Wed, Jun 05, 2013 at 11:31:44PM +0200, Maxime Ripard wrote:
> Hi Wolfram,
> 
> On Wed, Jun 05, 2013 at 03:39:47PM +0200, Wolfram Sang wrote:
> > 
> > > +#define SUNXI_I2C_ADDR_REG   (0x00)
> > > +#define SUNXI_I2C_ADDR_ADDR(v)   ((v & 0x7f) << 1)
> > > +#define SUNXI_I2C_XADDR_REG  (0x04)
> > > +#define SUNXI_I2C_DATA_REG   (0x08)
> > > +#define SUNXI_I2C_CNTR_REG   (0x0c)
> > > +#define SUNXI_I2C_CNTR_ASSERT_ACKBIT(2)
> > > +#define SUNXI_I2C_CNTR_INT_FLAG  BIT(3)
> > > +#define SUNXI_I2C_CNTR_MASTER_STOP   BIT(4)
> > > +#define SUNXI_I2C_CNTR_MASTER_START  BIT(5)
> > > +#define SUNXI_I2C_CNTR_BUS_ENABLEBIT(6)
> > > +#define SUNXI_I2C_CNTR_INT_ENABLEBIT(7)
> > > +#define SUNXI_I2C_STA_REG(0x10)
> > > +#define SUNXI_I2C_STA_BUS_ERROR  (0x00)
> > > +#define SUNXI_I2C_STA_START  (0x08)
> > > +#define SUNXI_I2C_STA_START_REPEAT   (0x10)
> > > +#define SUNXI_I2C_STA_MASTER_WADDR_ACK   (0x18)
> > > +#define SUNXI_I2C_STA_MASTER_WADDR_NAK   (0x20)
> > > +#define SUNXI_I2C_STA_MASTER_DATA_SENT_ACK   (0x28)
> > > +#define SUNXI_I2C_STA_MASTER_DATA_SENT_NAK   (0x30)
> > > +#define SUNXI_I2C_STA_MASTER_RADDR_ACK   (0x40)
> > > +#define SUNXI_I2C_STA_MASTER_RADDR_NAK   (0x48)
> > > +#define SUNXI_I2C_STA_MASTER_DATA_RECV_ACK   (0x50)
> > > +#define SUNXI_I2C_STA_MASTER_DATA_RECV_NAK   (0x58)
> > > +#define SUNXI_I2C_CCR_REG(0x14)
> > > +#define SUNXI_I2C_CCR_DIV_N(val) (val & 0x3)
> > > +#define SUNXI_I2C_CCR_DIV_M(val) ((val & 0xf) << 3)
> > > +#define SUNXI_I2C_SRST_REG   (0x18)
> > > +#define SUNXI_I2C_SRST_RESET BIT(0)
> > > +#define SUNXI_I2C_EFR_REG(0x1c)
> > > +#define SUNXI_I2C_LCR_REG(0x20)
> > > +
> > > +#define SUNXI_I2C_DONE   BIT(0)
> > > +#define SUNXI_I2C_ERROR  BIT(1)
> > > +#define SUNXI_I2C_NAKBIT(2)
> > > +#define SUNXI_I2C_BUS_ERROR  BIT(3)
> > 
> > The register set looks similar to i2c-mv64xxx.c. Has it been considered
> > to merge the two drivers?
> 
> Hmm, right, the logic seems surprisingly similar. I wasn't aware of such
> a driver, so I didn't put much thought into merging them :)

Too late now, yet grepping for specific register names is a good thing
to do before writing a new driber.

> However, the register layout is quite different enough to be quite
> painful to support: the XADDR register here seems to be equivalent to
> the Marvell's EXT_SLAVE_ADDR register, yet in the allwinner case, the
> offset is 0x4, while it's 0x1c in Marvell's case, which offsets all the
> other registers obviously. The status and clock registers in marvell
> case looks to be merged together, while they are separate registers
> here, etc.
> 
> I guess it would make the code much more complicated for such simple
> drivers, so I wouldn't push too much for merging, but I guess it's your
> call.

The thing is: If there is a bug/errata found in one driver, it goes
unnoticed that it probably should be fixed in the other, too. This is
why sharing logic is a good thing from a maintenance view.

Register offsets are annoying, but one can handle them. There are
examples in the kernel tree. If some registers are totally different
they often can be encapsulated in specific functions processing them.

So, I'd like to ask you if you can prepare a list of what would be
needed to merge this support into mv64xxx? Then I get a better picture
of what way to go. Can you do this?

Thanks,

   Wolfram



signature.asc
Description: Digital signature


Re: i2c: introduce i2c helper i2c_find_client_by_name()

2013-06-11 Thread Bin Gao
On Sun, Jun 09, 2013 at 10:53:35PM +0300, Andy Shevchenko wrote:
> Please, try to avoid top posting in the future emails.
> 
> On Fri, Jun 7, 2013 at 12:26 AM, Bin Gao  wrote:
> > With v4l2, the camera sensor i2c devices are taken over by v4l2 master
> > driver, i.e. ISP driver, and are not expected to be accessed from
> > user sapce by ioctl. So ISP driver has to register them by itself to
> > get all related information for further communication. Please check
> > v4l2_i2c_new_subdev_board() in drivers/media/video/v4l2-common.c for 
> > details.
> 
> Yes, this is legacy approach for non-DT/non-ACPI5 platforms.

So what's the new approach for DT/ACPI5 platforms?

> 
> > The platform code can definitely disallow calling i2c_register_board_info()
> > to register them but keep the i2c devices list and then let ISP driver 
> > register
> > them. But, problems come when a single kernel is going to run on two 
> > platforms
> > - one platform has ACPI5 and the other has SFI.
> 
> You have to forget about SFI. Your ISP subdevices use plain platform
> data anyway.

Why have to forget about SFI which is supported by upstream kernel?

> In ACPI 5 case v4l2 framework must be extended to cover that case.

To extend v4l2 framework is one of the options, but this helper is also one
option.

> > The dynamic unregister and then
> > register based on this new helper will not have dependency on firmware 
> > interface -
> > the same platform code will work for all platforms.
> 
> It's not a care of the ISP driver. I think you have to talk to Sakari
> about your issues.

We have to do it in the ISP driver if we want one binary kernel
supporting multiple platforms.

> > On Thu, Jun 06, 2013 at 11:32:06PM +0300, Andy Shevchenko wrote:
> >> On Thu, Jun 6, 2013 at 9:33 PM, Bin Gao  wrote:
> >> > There is a requirement to get the i2c_client pointer dynamically without
> >> > knowing the bus and slave address. But we do know the client name,
> >> > i.e. the name in the i2c_board_info. This patch is to fit this 
> >> > requirement.
> >> >
> >> > A good example is that an ISP(Imaging Signal Processor) driver needs
> >> > register i2c camera sensor devices via v4l2, so it has to unregister
> >> > all i2c clients that were previously registered by calling
> >> > i2c_register_board_info(), and then re-register. For this case we
> >> > can use this helper to get i2c_client by passing the client name.
> >>
> >> Why ISP driver would like to register sensor drivers in the first place?
> >> That seems the task of platform code, or DT, or ACPI5
> >>
> >> Why do you need to re-register them at run time?
> 
> --
> With Best Regards,
> Andy Shevchenko
--
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: sirf: move driver init from module_init to subsys_initcall

2013-06-11 Thread Mark Brown
On Tue, Jun 11, 2013 at 07:13:33PM +0800, Barry Song wrote:
> 2013/6/11 Mark Brown :

> > It's not that people don't care about these users, it's that people
> > don't care too much about out of tree users.  Part of the goal here is
> > to convince people working on such applications that they should make
> > mainline better for everyone so that you don't need external code to
> > make the kernel useful.

> Mark, then this is really confusing me.
> for this reason, the target should be making this out-of-tree stuff be
> inside the tree. to make i2c-sirf "mainline better and not need
> external code", the target should be making this external code not
> external but become internal. otherwise, my mainline users will always
> need this external code.

It's not just this one bit of code that they're relying on, this also
gets built on with other things that are also out of tree.  The thing
we need to do is figure out what problem the solution as a whole is
fixing and then make sure that mainline can deal with that.


signature.asc
Description: Digital signature


Re: [PATCH] i2c: sirf: move driver init from module_init to subsys_initcall

2013-06-11 Thread Barry Song
2013/6/11 Mark Brown :
> On Tue, Jun 11, 2013 at 09:14:41AM +0800, Barry Song wrote:
>
>> the mainline idea you mentioned is that you don't care about any early
>> device, which means some devices want to start earlier than others in
>> some real automative user scenerioes.
>> but i think another important idea is that mainline codes come from
>> branches of different vendors and serve final users of those drivers,
>> but not only make source codes no difference to all environments. the
>> auto users i2c-sirf serves, here, actually means some differences with
>> PC/tablet/mobilephone. we don't make codes good-looking by losing
>> functionality and not close to final users.
>
> It's not that people don't care about these users, it's that people
> don't care too much about out of tree users.  Part of the goal here is
> to convince people working on such applications that they should make
> mainline better for everyone so that you don't need external code to
> make the kernel useful.

Mark, then this is really confusing me.
for this reason, the target should be making this out-of-tree stuff be
inside the tree. to make i2c-sirf "mainline better and not need
external code", the target should be making this external code not
external but become internal. otherwise, my mainline users will always
need this external code.

-barry
--
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: sirf: move driver init from module_init to subsys_initcall

2013-06-11 Thread Mark Brown
On Tue, Jun 11, 2013 at 09:14:41AM +0800, Barry Song wrote:

> the mainline idea you mentioned is that you don't care about any early
> device, which means some devices want to start earlier than others in
> some real automative user scenerioes.
> but i think another important idea is that mainline codes come from
> branches of different vendors and serve final users of those drivers,
> but not only make source codes no difference to all environments. the
> auto users i2c-sirf serves, here, actually means some differences with
> PC/tablet/mobilephone. we don't make codes good-looking by losing
> functionality and not close to final users.

It's not that people don't care about these users, it's that people
don't care too much about out of tree users.  Part of the goal here is
to convince people working on such applications that they should make
mainline better for everyone so that you don't need external code to
make the kernel useful.


signature.asc
Description: Digital signature