* Re: [PATCH v3 2/5] i2c: designware-baytrail: Pass dw_i2c_dev into helper functions
From: Jarkko Nikula @ 2016-12-12 14:02 UTC (permalink / raw)
To: Hans de Goede, Wolfram Sang
Cc: Andy Shevchenko, Mika Westerberg, Takashi Iwai,
russianneuromancer @ ya . ru, Vincent Gerris, linux-i2c
In-Reply-To: <20161210224350.10290-2-hdegoede@redhat.com>
On 11.12.2016 00:43, Hans de Goede wrote:
> Pass dw_i2c_dev into the helper functions, this is a preparation patch
> for the punit semaphore fixes done in the other patches in this set.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> Reviewed-by: Takashi Iwai <tiwai@suse.de>
> Tested-by: Takashi Iwai <tiwai@suse.de>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
> drivers/i2c/busses/i2c-designware-baytrail.c | 16 ++++++++--------
> 1 file changed, 8 insertions(+), 8 deletions(-)
>
Acked-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
^ permalink raw reply
* Re: [PATCH v3 1/5] i2c: designware: Rename accessor_flags to flags
From: Jarkko Nikula @ 2016-12-12 14:02 UTC (permalink / raw)
To: Hans de Goede, Wolfram Sang
Cc: Andy Shevchenko, Mika Westerberg, Takashi Iwai,
russianneuromancer @ ya . ru, Vincent Gerris, linux-i2c
In-Reply-To: <20161210224350.10290-1-hdegoede@redhat.com>
On 11.12.2016 00:43, Hans de Goede wrote:
> Rename accessor_flags to flags, so that we can use the field for
> other flags too. This is a preparation patch for adding cherrytrail
> support to the punit semaphore code.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
> drivers/i2c/busses/i2c-designware-core.c | 14 +++++++-------
> drivers/i2c/busses/i2c-designware-core.h | 2 +-
> drivers/i2c/busses/i2c-designware-platdrv.c | 2 +-
> 3 files changed, 9 insertions(+), 9 deletions(-)
>
Acked-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
^ permalink raw reply
* Re: [PATCH v6 4/9] dt-bindings: iio: iio-mux: document iio-mux bindings
From: Peter Rosin @ 2016-12-12 12:18 UTC (permalink / raw)
To: Jonathan Cameron, Rob Herring
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA, Wolfram Sang, Mark Rutland,
Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler,
Jonathan Corbet, Arnd Bergmann, Greg Kroah-Hartman,
linux-i2c-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-iio-u79uwXL29TY76Z2rM5mHXA,
linux-doc-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <d2726499-4034-8d5d-4cff-61da86af5add-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
On 2016-12-10 19:21, Jonathan Cameron wrote:
> On 06/12/16 09:18, Peter Rosin wrote:
>> On 2016-12-06 00:26, Rob Herring wrote:
>>> On Wed, Nov 30, 2016 at 09:16:58AM +0100, Peter Rosin wrote:
>>>> Signed-off-by: Peter Rosin <peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>
>>>> ---
>>>> .../bindings/iio/multiplexer/iio-mux.txt | 40 ++++++++++++++++++++++
>>>> MAINTAINERS | 6 ++++
>>>> 2 files changed, 46 insertions(+)
>>>> create mode 100644 Documentation/devicetree/bindings/iio/multiplexer/iio-mux.txt
>>>
>>> I'm still not convinced about this binding, but don't really have more
>>> comments ATM. Sending 6 versions in 2 weeks or so doesn't really help
>>> either.
>>
>> Sorry about the noise, I'll try to be more careful going forward. On
>> the flip side, I haven't touched the code since v6.
>>
>> I don't see how bindings that are as flexible as the current (and
>> original) phandle link between the mux consumer and the mux controller
>> would look, and at the same time be simpler to understand. You need
>> to be able to refer to a mux controller from several mux consumers, and
>> you need to support several mux controllers in one node (the ADG792A
>> case). And, AFAICT, the complex case wasn't really the problem, it was
>> that it is overly complex to describe the simple case of one mux
>> consumer and one mux controller. But in your comment for v2 [1] you
>> said that I was working around limitations with shared GPIO pins. But
>> solving that in the GPIO subsystem would not solve all that the
>> phandle approach is solving, since you would not have support for
>> ADG792A (or other non-GPIO controlled muxes). So, I think listing
>> the gpio pins inside the mux consumer node is a non-starter, the mux
>> controller has to live in its own node with its own compatible.
>>
>> Would you be happier if I managed to marry the phandle approach with
>> the option of having the mux controller as a child node of the mux
>> consumer for the simple case?
>>
>> I added an example at the end of this message (the same as the first
>> example in v4 [2], at least in principle) for easy comparison between
>> the phandle and the controller-in-child-node approaches. I can't say
>> that I personally find the difference all that significant, and do not
>> think it is worth it. As I see it, the "simple option" would just muddy
>> the waters...
>>
>> [1] http://marc.info/?l=linux-kernel&m=147948334204795&w=2
>> [2] http://marc.info/?l=linux-kernel&m=148001364904240&w=2
>>
>>>> diff --git a/Documentation/devicetree/bindings/iio/multiplexer/iio-mux.txt b/Documentation/devicetree/bindings/iio/multiplexer/iio-mux.txt
>>>> new file mode 100644
>>>> index 000000000000..8080cf790d82
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/iio/multiplexer/iio-mux.txt
>>>> @@ -0,0 +1,40 @@
>>>> +IIO multiplexer bindings
>>>> +
>>>> +If a multiplexer is used to select which hardware signal is fed to
>>>> +e.g. an ADC channel, these bindings describe that situation.
>>>> +
>>>> +Required properties:
>>>> +- compatible : "iio-mux"
>>>
>>> This is a Linuxism. perhaps "adc-mux".
>>
>> No, that's not general enough, it could just as well be used to mux a
>> temperature sensor. Or whatever. Hmmm, given that "iio-mux" is bad, perhaps
>> "io-channel-mux" is better? That matches the io-channels property used to
>> refer to the parent channel.
> analog-mux maybe? Makes more sense out of context (though with io-channels defined on
> the next line you have plenty of context here ;)
Not that I care all that much about the name, but that doesn't really
fit if you take e.g. an IIO_INDEX channel. That sounds entirely non-analog
to me, but what do I know? Maybe that example doesn't make sense for some
reason, but I can't help but think that there will be some non-analog
channel in the future, if there isn't one already.
So, my preference is io-channel-mux, as that matches the previous dt
naming for what is muxed. But that's just my opinion, if I'm told that
it should be something else, then that's ok.
I'm more worried about other aspects, such as how to get reviewers and who
is going to take the core mux patches and what tree they are going to be
merged into etc. That is, if this series is going anywhere at all or if
someone is going to put up a road-block for some reason...
Cheers,
peda
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v3 5/5] i2c: designware-baytrail: Add support for cherrytrail
From: Andy Shevchenko @ 2016-12-12 11:42 UTC (permalink / raw)
To: Hans de Goede, Jarkko Nikula, Wolfram Sang
Cc: Mika Westerberg, Takashi Iwai, russianneuromancer @ ya . ru,
Vincent Gerris, linux-i2c
In-Reply-To: <8b835a2c-a54c-8539-2ffd-28a480459a2b@redhat.com>
On Mon, 2016-12-12 at 11:48 +0100, Hans de Goede wrote:
> Hi,
>
> On 12-12-16 11:31, Andy Shevchenko wrote:
> > On Sat, 2016-12-10 at 23:43 +0100, Hans de Goede wrote:
> > > The cherrytrail punit has the pmic i2c bus access semaphore at a
> > > different register address.
> > >
> > > Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> > > Reviewed-by: Takashi Iwai <tiwai@suse.de>
> > > Tested-by: Takashi Iwai <tiwai@suse.de>
> >
> > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> >
> > One small nit: you are using CHV abbreviation for CherryTrail, we
> > are
> > consider CHT is a right one, CHV for CherryView.
>
> I thought that cherryview and cherrytrail where 2 names for
> the same chip ?
TBH, I don't know the difference but it is there, Like SoC -> platform.
In any case, just to be consistent with your code below. Everywhere else
you used cherrytrail.
>
> Regards,
>
> Hans
>
>
> >
> > > ---
> > > Changes in v2:
> > > -Adjust for accessor_flags -> flags rename
> > > -Add flags field to struct dw_pci_controller
> > > -Add get_sem_addr() helper replacing MODEL_CHERRYTRAIL flag
> > > checking
> > > in
> > > PUNIT_SEMAPHORE macro
> > > Changes in v3:
> > > -Add a gap between ACCESS_* and MODEL_* flags as reserved space
> > > for
> > > future ACCESS_* flags
> > > ---
> > > drivers/i2c/busses/i2c-designware-baytrail.c | 22
> > > +++++++++++++++++
> > > -----
> > > drivers/i2c/busses/i2c-designware-core.h | 2 ++
> > > drivers/i2c/busses/i2c-designware-pcidrv.c | 26
> > > +++++++++++++++++++-------
> > > drivers/i2c/busses/i2c-designware-platdrv.c | 2 +-
> > > 4 files changed, 39 insertions(+), 13 deletions(-)
> > >
> > > diff --git a/drivers/i2c/busses/i2c-designware-baytrail.c
> > > b/drivers/i2c/busses/i2c-designware-baytrail.c
> > > index 997d048..456a236 100644
> > > --- a/drivers/i2c/busses/i2c-designware-baytrail.c
> > > +++ b/drivers/i2c/busses/i2c-designware-baytrail.c
> > > @@ -24,13 +24,23 @@
> > >
> > > #define SEMAPHORE_TIMEOUT 100
> > > #define PUNIT_SEMAPHORE 0x7
> > > +#define PUNIT_SEMAPHORE_CHV 0x10e
> > > #define PUNIT_SEMAPHORE_BIT BIT(0)
> > > #define PUNIT_SEMAPHORE_ACQUIRE BIT(1)
> > >
> > > static unsigned long acquired;
> > >
> > > +static u32 get_sem_addr(struct dw_i2c_dev *dev)
> > > +{
> > > + if (dev->flags & MODEL_CHERRYTRAIL)
> > > + return PUNIT_SEMAPHORE_CHV;
> > > + else
> > > + return PUNIT_SEMAPHORE;
> > > +}
> > > +
> > > static int get_sem(struct dw_i2c_dev *dev, u32 *sem)
> > > {
> > > + u32 addr = get_sem_addr(dev);
> > > u32 data;
> > > int ret;
> > >
> > > @@ -41,7 +51,7 @@ static int get_sem(struct dw_i2c_dev *dev, u32
> > > *sem)
> > > */
> > > pm_qos_update_request(&dev->pm_qos, 0);
> > >
> > > - ret = iosf_mbi_read(BT_MBI_UNIT_PMC, MBI_REG_READ,
> > > PUNIT_SEMAPHORE, &data);
> > > + ret = iosf_mbi_read(BT_MBI_UNIT_PMC, MBI_REG_READ, addr,
> > > &data);
> > > if (ret) {
> > > dev_err(dev->dev, "iosf failed to read punit
> > > semaphore\n");
> > > return ret;
> > > @@ -54,15 +64,16 @@ static int get_sem(struct dw_i2c_dev *dev, u32
> > > *sem)
> > >
> > > static void reset_semaphore(struct dw_i2c_dev *dev)
> > > {
> > > + u32 addr = get_sem_addr(dev);
> > > u32 data;
> > >
> > > - if (iosf_mbi_read(BT_MBI_UNIT_PMC, MBI_REG_READ,
> > > PUNIT_SEMAPHORE, &data)) {
> > > + if (iosf_mbi_read(BT_MBI_UNIT_PMC, MBI_REG_READ, addr,
> > > &data)) {
> > > dev_err(dev->dev, "iosf failed to reset punit
> > > semaphore during read\n");
> > > return;
> > > }
> > >
> > > data &= ~PUNIT_SEMAPHORE_BIT;
> > > - if (iosf_mbi_write(BT_MBI_UNIT_PMC, MBI_REG_WRITE,
> > > PUNIT_SEMAPHORE, data))
> > > + if (iosf_mbi_write(BT_MBI_UNIT_PMC, MBI_REG_WRITE, addr,
> > > data))
> > > dev_err(dev->dev, "iosf failed to reset punit
> > > semaphore during write\n");
> > >
> > > pm_qos_update_request(&dev->pm_qos,
> > > PM_QOS_DEFAULT_VALUE);
> > > @@ -70,6 +81,7 @@ static void reset_semaphore(struct dw_i2c_dev
> > > *dev)
> > >
> > > static int baytrail_i2c_acquire(struct dw_i2c_dev *dev)
> > > {
> > > + u32 addr = get_sem_addr(dev);
> > > u32 sem = PUNIT_SEMAPHORE_ACQUIRE;
> > > int ret;
> > > unsigned long start, end;
> > > @@ -83,7 +95,7 @@ static int baytrail_i2c_acquire(struct
> > > dw_i2c_dev
> > > *dev)
> > > return 0;
> > >
> > > /* host driver writes to side band semaphore register */
> > > - ret = iosf_mbi_write(BT_MBI_UNIT_PMC, MBI_REG_WRITE,
> > > PUNIT_SEMAPHORE, sem);
> > > + ret = iosf_mbi_write(BT_MBI_UNIT_PMC, MBI_REG_WRITE,
> > > addr,
> > > sem);
> > > if (ret) {
> > > dev_err(dev->dev, "iosf punit semaphore request
> > > failed\n");
> > > return ret;
> > > @@ -107,7 +119,7 @@ static int baytrail_i2c_acquire(struct
> > > dw_i2c_dev
> > > *dev)
> > > dev_err(dev->dev, "punit semaphore timed out,
> > > resetting\n");
> > > reset_semaphore(dev);
> > >
> > > - ret = iosf_mbi_read(BT_MBI_UNIT_PMC, MBI_REG_READ,
> > > PUNIT_SEMAPHORE, &sem);
> > > + ret = iosf_mbi_read(BT_MBI_UNIT_PMC, MBI_REG_READ, addr,
> > > &sem);
> > > if (ret)
> > > dev_err(dev->dev, "iosf failed to read punit
> > > semaphore\n");
> > > else
> > > diff --git a/drivers/i2c/busses/i2c-designware-core.h
> > > b/drivers/i2c/busses/i2c-designware-core.h
> > > index 47d284c..2db3177 100644
> > > --- a/drivers/i2c/busses/i2c-designware-core.h
> > > +++ b/drivers/i2c/busses/i2c-designware-core.h
> > > @@ -127,6 +127,8 @@ struct dw_i2c_dev {
> > > #define ACCESS_16BIT 0x00000002
> > > #define ACCESS_INTR_MASK 0x00000004
> > >
> > > +#define MODEL_CHERRYTRAIL 0x00000100
> > > +
> > > extern int i2c_dw_init(struct dw_i2c_dev *dev);
> > > extern void i2c_dw_disable(struct dw_i2c_dev *dev);
> > > extern void i2c_dw_disable_int(struct dw_i2c_dev *dev);
> > > diff --git a/drivers/i2c/busses/i2c-designware-pcidrv.c
> > > b/drivers/i2c/busses/i2c-designware-pcidrv.c
> > > index 96f8230..4e53a9f 100644
> > > --- a/drivers/i2c/busses/i2c-designware-pcidrv.c
> > > +++ b/drivers/i2c/busses/i2c-designware-pcidrv.c
> > > @@ -45,6 +45,7 @@ enum dw_pci_ctl_id_t {
> > > medfield,
> > > merrifield,
> > > baytrail,
> > > + cherrytrail,
> > > haswell,
> > > };
> > >
> > > @@ -63,6 +64,7 @@ struct dw_pci_controller {
> > > u32 rx_fifo_depth;
> > > u32 clk_khz;
> > > u32 functionality;
> > > + u32 flags;
> > > struct dw_scl_sda_cfg *scl_sda_cfg;
> > > int (*setup)(struct pci_dev *pdev, struct
> > > dw_pci_controller
> > > *c);
> > > };
> > > @@ -174,6 +176,15 @@ static struct dw_pci_controller
> > > dw_pci_controllers[] = {
> > > .functionality = I2C_FUNC_10BIT_ADDR,
> > > .scl_sda_cfg = &hsw_config,
> > > },
> > > + [cherrytrail] = {
> > > + .bus_num = -1,
> > > + .bus_cfg = INTEL_MID_STD_CFG |
> > > DW_IC_CON_SPEED_FAST,
> > > + .tx_fifo_depth = 32,
> > > + .rx_fifo_depth = 32,
> > > + .functionality = I2C_FUNC_10BIT_ADDR,
> > > + .flags = MODEL_CHERRYTRAIL,
> > > + .scl_sda_cfg = &byt_config,
> > > + },
> > > };
> > >
> > > #ifdef CONFIG_PM
> > > @@ -241,6 +252,7 @@ static int i2c_dw_pci_probe(struct pci_dev
> > > *pdev,
> > > dev->base = pcim_iomap_table(pdev)[0];
> > > dev->dev = &pdev->dev;
> > > dev->irq = pdev->irq;
> > > + dev->flags |= controller->flags;
> > >
> > > if (controller->setup) {
> > > r = controller->setup(pdev, controller);
> > > @@ -321,13 +333,13 @@ static const struct pci_device_id
> > > i2_designware_pci_ids[] = {
> > > { PCI_VDEVICE(INTEL, 0x9c61), haswell },
> > > { PCI_VDEVICE(INTEL, 0x9c62), haswell },
> > > /* Braswell / Cherrytrail */
> > > - { PCI_VDEVICE(INTEL, 0x22C1), baytrail },
> > > - { PCI_VDEVICE(INTEL, 0x22C2), baytrail },
> > > - { PCI_VDEVICE(INTEL, 0x22C3), baytrail },
> > > - { PCI_VDEVICE(INTEL, 0x22C4), baytrail },
> > > - { PCI_VDEVICE(INTEL, 0x22C5), baytrail },
> > > - { PCI_VDEVICE(INTEL, 0x22C6), baytrail },
> > > - { PCI_VDEVICE(INTEL, 0x22C7), baytrail },
> > > + { PCI_VDEVICE(INTEL, 0x22C1), cherrytrail },
> > > + { PCI_VDEVICE(INTEL, 0x22C2), cherrytrail },
> > > + { PCI_VDEVICE(INTEL, 0x22C3), cherrytrail },
> > > + { PCI_VDEVICE(INTEL, 0x22C4), cherrytrail },
> > > + { PCI_VDEVICE(INTEL, 0x22C5), cherrytrail },
> > > + { PCI_VDEVICE(INTEL, 0x22C6), cherrytrail },
> > > + { PCI_VDEVICE(INTEL, 0x22C7), cherrytrail },
> > > { 0,}
> > > };
> > > MODULE_DEVICE_TABLE(pci, i2_designware_pci_ids);
> > > diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c
> > > b/drivers/i2c/busses/i2c-designware-platdrv.c
> > > index 6d72929..589f07e 100644
> > > --- a/drivers/i2c/busses/i2c-designware-platdrv.c
> > > +++ b/drivers/i2c/busses/i2c-designware-platdrv.c
> > > @@ -123,7 +123,7 @@ static const struct acpi_device_id
> > > dw_i2c_acpi_match[] = {
> > > { "INT3432", 0 },
> > > { "INT3433", 0 },
> > > { "80860F41", 0 },
> > > - { "808622C1", 0 },
> > > + { "808622C1", MODEL_CHERRYTRAIL },
> > > { "AMD0010", ACCESS_INTR_MASK },
> > > { "AMDI0010", ACCESS_INTR_MASK },
> > > { "AMDI0510", 0 },
--
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy
^ permalink raw reply
* Re: [PATCH v5 1/2] i2c: aspeed: added driver for Aspeed I2C
From: Kachalov Anton @ 2016-12-12 11:06 UTC (permalink / raw)
To: Wolfram Sang, Brendan Higgins
Cc: vz@mleia.com, clg@kaod.org, robh+dt@kernel.org,
mark.rutland@arm.com, linux-i2c@vger.kernel.org,
devicetree@vger.kernel.org, joel@jms.id.au,
openbmc@lists.ozlabs.org
In-Reply-To: <20161211222622.GK2552@katana>
12.12.2016, 01:26, "Wolfram Sang" <wsa@the-dreams.de>:
> On Tue, Nov 29, 2016 at 05:00:17PM -0800, Brendan Higgins wrote:
>> Added initial master and slave support for Aspeed I2C controller.
>> Supports fourteen busses present in ast24xx and ast25xx BMC SoCs by
>> Aspeed.
>>
>> Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
>
> BTW first the bindings patch please, then the driver.
>
> And one seperate question I just stumbled over:
>
>> + /* Switch from master mode to slave mode. */
>> + func_ctrl_reg_val = aspeed_i2c_read(bus, ASPEED_I2C_FUN_CTRL_REG);
>> + func_ctrl_reg_val &= ~ASPEED_I2CD_MASTER_EN;
>> + func_ctrl_reg_val |= ASPEED_I2CD_SLAVE_EN;
>> + aspeed_i2c_write(bus, func_ctrl_reg_val, ASPEED_I2C_FUN_CTRL_REG);
>
> Can't the hardware work both as master and slave on the same bus?
The hardware can work as master and slave on the same bus. This is how IPMB over i2c works on Aspeed.
^ permalink raw reply
* Re: [PATCH v5 1/2] i2c: aspeed: added driver for Aspeed I2C
From: Wolfram Sang @ 2016-12-12 11:10 UTC (permalink / raw)
To: Kachalov Anton
Cc: Brendan Higgins, vz-ChpfBGZJDbMAvxtiuMwx3w@public.gmane.org,
clg-Bxea+6Xhats@public.gmane.org,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
mark.rutland-5wv7dgnIgG8@public.gmane.org,
linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
joel-U3u1mxZcP9KHXe+LvDLADg@public.gmane.org,
openbmc-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
In-Reply-To: <825541481540788-Mr5SgJCofHtxpj1cXAZ9Bg@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 680 bytes --]
> >> + /* Switch from master mode to slave mode. */
> >> + func_ctrl_reg_val = aspeed_i2c_read(bus, ASPEED_I2C_FUN_CTRL_REG);
> >> + func_ctrl_reg_val &= ~ASPEED_I2CD_MASTER_EN;
> >> + func_ctrl_reg_val |= ASPEED_I2CD_SLAVE_EN;
> >> + aspeed_i2c_write(bus, func_ctrl_reg_val, ASPEED_I2C_FUN_CTRL_REG);
> >
> > Can't the hardware work both as master and slave on the same bus?
>
> The hardware can work as master and slave on the same bus. This is how IPMB over i2c works on Aspeed.
Thanks! Then the driver should support this. Maybe it is an idea to
first upstream the master support and add the slave support
incrementally?
Regards,
Wolfram
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH v3 5/5] i2c: designware-baytrail: Add support for cherrytrail
From: Hans de Goede @ 2016-12-12 10:48 UTC (permalink / raw)
To: Andy Shevchenko, Jarkko Nikula, Wolfram Sang
Cc: Mika Westerberg, Takashi Iwai, russianneuromancer @ ya . ru,
Vincent Gerris, linux-i2c
In-Reply-To: <1481538690.7188.18.camel@linux.intel.com>
Hi,
On 12-12-16 11:31, Andy Shevchenko wrote:
> On Sat, 2016-12-10 at 23:43 +0100, Hans de Goede wrote:
>> The cherrytrail punit has the pmic i2c bus access semaphore at a
>> different register address.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> Reviewed-by: Takashi Iwai <tiwai@suse.de>
>> Tested-by: Takashi Iwai <tiwai@suse.de>
>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>
> One small nit: you are using CHV abbreviation for CherryTrail, we are
> consider CHT is a right one, CHV for CherryView.
I thought that cherryview and cherrytrail where 2 names for
the same chip ?
Regards,
Hans
>
>> ---
>> Changes in v2:
>> -Adjust for accessor_flags -> flags rename
>> -Add flags field to struct dw_pci_controller
>> -Add get_sem_addr() helper replacing MODEL_CHERRYTRAIL flag checking
>> in
>> PUNIT_SEMAPHORE macro
>> Changes in v3:
>> -Add a gap between ACCESS_* and MODEL_* flags as reserved space for
>> future ACCESS_* flags
>> ---
>> drivers/i2c/busses/i2c-designware-baytrail.c | 22 +++++++++++++++++
>> -----
>> drivers/i2c/busses/i2c-designware-core.h | 2 ++
>> drivers/i2c/busses/i2c-designware-pcidrv.c | 26
>> +++++++++++++++++++-------
>> drivers/i2c/busses/i2c-designware-platdrv.c | 2 +-
>> 4 files changed, 39 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/i2c/busses/i2c-designware-baytrail.c
>> b/drivers/i2c/busses/i2c-designware-baytrail.c
>> index 997d048..456a236 100644
>> --- a/drivers/i2c/busses/i2c-designware-baytrail.c
>> +++ b/drivers/i2c/busses/i2c-designware-baytrail.c
>> @@ -24,13 +24,23 @@
>>
>> #define SEMAPHORE_TIMEOUT 100
>> #define PUNIT_SEMAPHORE 0x7
>> +#define PUNIT_SEMAPHORE_CHV 0x10e
>> #define PUNIT_SEMAPHORE_BIT BIT(0)
>> #define PUNIT_SEMAPHORE_ACQUIRE BIT(1)
>>
>> static unsigned long acquired;
>>
>> +static u32 get_sem_addr(struct dw_i2c_dev *dev)
>> +{
>> + if (dev->flags & MODEL_CHERRYTRAIL)
>> + return PUNIT_SEMAPHORE_CHV;
>> + else
>> + return PUNIT_SEMAPHORE;
>> +}
>> +
>> static int get_sem(struct dw_i2c_dev *dev, u32 *sem)
>> {
>> + u32 addr = get_sem_addr(dev);
>> u32 data;
>> int ret;
>>
>> @@ -41,7 +51,7 @@ static int get_sem(struct dw_i2c_dev *dev, u32 *sem)
>> */
>> pm_qos_update_request(&dev->pm_qos, 0);
>>
>> - ret = iosf_mbi_read(BT_MBI_UNIT_PMC, MBI_REG_READ,
>> PUNIT_SEMAPHORE, &data);
>> + ret = iosf_mbi_read(BT_MBI_UNIT_PMC, MBI_REG_READ, addr,
>> &data);
>> if (ret) {
>> dev_err(dev->dev, "iosf failed to read punit
>> semaphore\n");
>> return ret;
>> @@ -54,15 +64,16 @@ static int get_sem(struct dw_i2c_dev *dev, u32
>> *sem)
>>
>> static void reset_semaphore(struct dw_i2c_dev *dev)
>> {
>> + u32 addr = get_sem_addr(dev);
>> u32 data;
>>
>> - if (iosf_mbi_read(BT_MBI_UNIT_PMC, MBI_REG_READ,
>> PUNIT_SEMAPHORE, &data)) {
>> + if (iosf_mbi_read(BT_MBI_UNIT_PMC, MBI_REG_READ, addr,
>> &data)) {
>> dev_err(dev->dev, "iosf failed to reset punit
>> semaphore during read\n");
>> return;
>> }
>>
>> data &= ~PUNIT_SEMAPHORE_BIT;
>> - if (iosf_mbi_write(BT_MBI_UNIT_PMC, MBI_REG_WRITE,
>> PUNIT_SEMAPHORE, data))
>> + if (iosf_mbi_write(BT_MBI_UNIT_PMC, MBI_REG_WRITE, addr,
>> data))
>> dev_err(dev->dev, "iosf failed to reset punit
>> semaphore during write\n");
>>
>> pm_qos_update_request(&dev->pm_qos, PM_QOS_DEFAULT_VALUE);
>> @@ -70,6 +81,7 @@ static void reset_semaphore(struct dw_i2c_dev *dev)
>>
>> static int baytrail_i2c_acquire(struct dw_i2c_dev *dev)
>> {
>> + u32 addr = get_sem_addr(dev);
>> u32 sem = PUNIT_SEMAPHORE_ACQUIRE;
>> int ret;
>> unsigned long start, end;
>> @@ -83,7 +95,7 @@ static int baytrail_i2c_acquire(struct dw_i2c_dev
>> *dev)
>> return 0;
>>
>> /* host driver writes to side band semaphore register */
>> - ret = iosf_mbi_write(BT_MBI_UNIT_PMC, MBI_REG_WRITE,
>> PUNIT_SEMAPHORE, sem);
>> + ret = iosf_mbi_write(BT_MBI_UNIT_PMC, MBI_REG_WRITE, addr,
>> sem);
>> if (ret) {
>> dev_err(dev->dev, "iosf punit semaphore request
>> failed\n");
>> return ret;
>> @@ -107,7 +119,7 @@ static int baytrail_i2c_acquire(struct dw_i2c_dev
>> *dev)
>> dev_err(dev->dev, "punit semaphore timed out, resetting\n");
>> reset_semaphore(dev);
>>
>> - ret = iosf_mbi_read(BT_MBI_UNIT_PMC, MBI_REG_READ,
>> PUNIT_SEMAPHORE, &sem);
>> + ret = iosf_mbi_read(BT_MBI_UNIT_PMC, MBI_REG_READ, addr,
>> &sem);
>> if (ret)
>> dev_err(dev->dev, "iosf failed to read punit
>> semaphore\n");
>> else
>> diff --git a/drivers/i2c/busses/i2c-designware-core.h
>> b/drivers/i2c/busses/i2c-designware-core.h
>> index 47d284c..2db3177 100644
>> --- a/drivers/i2c/busses/i2c-designware-core.h
>> +++ b/drivers/i2c/busses/i2c-designware-core.h
>> @@ -127,6 +127,8 @@ struct dw_i2c_dev {
>> #define ACCESS_16BIT 0x00000002
>> #define ACCESS_INTR_MASK 0x00000004
>>
>> +#define MODEL_CHERRYTRAIL 0x00000100
>> +
>> extern int i2c_dw_init(struct dw_i2c_dev *dev);
>> extern void i2c_dw_disable(struct dw_i2c_dev *dev);
>> extern void i2c_dw_disable_int(struct dw_i2c_dev *dev);
>> diff --git a/drivers/i2c/busses/i2c-designware-pcidrv.c
>> b/drivers/i2c/busses/i2c-designware-pcidrv.c
>> index 96f8230..4e53a9f 100644
>> --- a/drivers/i2c/busses/i2c-designware-pcidrv.c
>> +++ b/drivers/i2c/busses/i2c-designware-pcidrv.c
>> @@ -45,6 +45,7 @@ enum dw_pci_ctl_id_t {
>> medfield,
>> merrifield,
>> baytrail,
>> + cherrytrail,
>> haswell,
>> };
>>
>> @@ -63,6 +64,7 @@ struct dw_pci_controller {
>> u32 rx_fifo_depth;
>> u32 clk_khz;
>> u32 functionality;
>> + u32 flags;
>> struct dw_scl_sda_cfg *scl_sda_cfg;
>> int (*setup)(struct pci_dev *pdev, struct dw_pci_controller
>> *c);
>> };
>> @@ -174,6 +176,15 @@ static struct dw_pci_controller
>> dw_pci_controllers[] = {
>> .functionality = I2C_FUNC_10BIT_ADDR,
>> .scl_sda_cfg = &hsw_config,
>> },
>> + [cherrytrail] = {
>> + .bus_num = -1,
>> + .bus_cfg = INTEL_MID_STD_CFG | DW_IC_CON_SPEED_FAST,
>> + .tx_fifo_depth = 32,
>> + .rx_fifo_depth = 32,
>> + .functionality = I2C_FUNC_10BIT_ADDR,
>> + .flags = MODEL_CHERRYTRAIL,
>> + .scl_sda_cfg = &byt_config,
>> + },
>> };
>>
>> #ifdef CONFIG_PM
>> @@ -241,6 +252,7 @@ static int i2c_dw_pci_probe(struct pci_dev *pdev,
>> dev->base = pcim_iomap_table(pdev)[0];
>> dev->dev = &pdev->dev;
>> dev->irq = pdev->irq;
>> + dev->flags |= controller->flags;
>>
>> if (controller->setup) {
>> r = controller->setup(pdev, controller);
>> @@ -321,13 +333,13 @@ static const struct pci_device_id
>> i2_designware_pci_ids[] = {
>> { PCI_VDEVICE(INTEL, 0x9c61), haswell },
>> { PCI_VDEVICE(INTEL, 0x9c62), haswell },
>> /* Braswell / Cherrytrail */
>> - { PCI_VDEVICE(INTEL, 0x22C1), baytrail },
>> - { PCI_VDEVICE(INTEL, 0x22C2), baytrail },
>> - { PCI_VDEVICE(INTEL, 0x22C3), baytrail },
>> - { PCI_VDEVICE(INTEL, 0x22C4), baytrail },
>> - { PCI_VDEVICE(INTEL, 0x22C5), baytrail },
>> - { PCI_VDEVICE(INTEL, 0x22C6), baytrail },
>> - { PCI_VDEVICE(INTEL, 0x22C7), baytrail },
>> + { PCI_VDEVICE(INTEL, 0x22C1), cherrytrail },
>> + { PCI_VDEVICE(INTEL, 0x22C2), cherrytrail },
>> + { PCI_VDEVICE(INTEL, 0x22C3), cherrytrail },
>> + { PCI_VDEVICE(INTEL, 0x22C4), cherrytrail },
>> + { PCI_VDEVICE(INTEL, 0x22C5), cherrytrail },
>> + { PCI_VDEVICE(INTEL, 0x22C6), cherrytrail },
>> + { PCI_VDEVICE(INTEL, 0x22C7), cherrytrail },
>> { 0,}
>> };
>> MODULE_DEVICE_TABLE(pci, i2_designware_pci_ids);
>> diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c
>> b/drivers/i2c/busses/i2c-designware-platdrv.c
>> index 6d72929..589f07e 100644
>> --- a/drivers/i2c/busses/i2c-designware-platdrv.c
>> +++ b/drivers/i2c/busses/i2c-designware-platdrv.c
>> @@ -123,7 +123,7 @@ static const struct acpi_device_id
>> dw_i2c_acpi_match[] = {
>> { "INT3432", 0 },
>> { "INT3433", 0 },
>> { "80860F41", 0 },
>> - { "808622C1", 0 },
>> + { "808622C1", MODEL_CHERRYTRAIL },
>> { "AMD0010", ACCESS_INTR_MASK },
>> { "AMDI0010", ACCESS_INTR_MASK },
>> { "AMDI0510", 0 },
>
^ permalink raw reply
* Re: [PATCH v3 4/5] i2c: designware-baytrail: Disallow the CPU to enter C6 or C7 while holding the punit semaphore
From: Andy Shevchenko @ 2016-12-12 10:37 UTC (permalink / raw)
To: Hans de Goede, Jarkko Nikula, Wolfram Sang
Cc: Mika Westerberg, Takashi Iwai, russianneuromancer @ ya . ru,
Vincent Gerris, linux-i2c
In-Reply-To: <20161210224350.10290-4-hdegoede@redhat.com>
On Sat, 2016-12-10 at 23:43 +0100, Hans de Goede wrote:
> On my cherrytrail tablet with axp288 pmic, just doing a bunch of
> repeated
> reads from the pmic, e.g. "i2cdump -y 14 0x34" would lookup the tablet
> in
> 1 - 3 runs guaranteed.
>
> This seems to be causes by the cpu trying to enter C6 or C7 while we
> hold
> the punit bus semaphore, at which point everything just hangs.
>
> Avoid this by the CPU to enter C6 or C7 before acquiring the punit bus
> semaphore.
Please, keep Len Brown in a loop for this patch. And perhaps someone
from our Graphics team. Maybe Imre Deak?
See my comments below.
>
> BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=109051
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> Changes in v2:
> -New patch in v2 of this set
> Changes in v3:
> -Change commit message and comment in the code from "force the CPU to
> C1"
> to "Disallow the CPU to enter C6 or C7", as the CPU may still be in
> either
> C0 or C1 with the request pm_qos
> ---
> drivers/i2c/busses/i2c-designware-baytrail.c | 12 ++++++++++++
> drivers/i2c/busses/i2c-designware-core.h | 3 +++
> drivers/i2c/busses/i2c-designware-platdrv.c | 3 +++
> 3 files changed, 18 insertions(+)
>
> diff --git a/drivers/i2c/busses/i2c-designware-baytrail.c
> b/drivers/i2c/busses/i2c-designware-baytrail.c
> index cf02222..997d048 100644
> --- a/drivers/i2c/busses/i2c-designware-baytrail.c
> +++ b/drivers/i2c/busses/i2c-designware-baytrail.c
> @@ -16,6 +16,7 @@
> #include <linux/acpi.h>
> #include <linux/i2c.h>
> #include <linux/interrupt.h>
> +#include <linux/pm_qos.h>
>
> #include <asm/iosf_mbi.h>
>
> @@ -33,6 +34,13 @@ static int get_sem(struct dw_i2c_dev *dev, u32
> *sem)
> u32 data;
> int ret;
>
> + /*
> + * Disallow the CPU to enter C6 or C7 state, entering these
> states
> + * requires the punit to talk to the pmic and if this happens
> while
> + * we're holding the semaphore, the SoC hangs.
> + */
> + pm_qos_update_request(&dev->pm_qos, 0);
> +
> ret = iosf_mbi_read(BT_MBI_UNIT_PMC, MBI_REG_READ,
> PUNIT_SEMAPHORE, &data);
> if (ret) {
> dev_err(dev->dev, "iosf failed to read punit
> semaphore\n");
> @@ -56,6 +64,8 @@ static void reset_semaphore(struct dw_i2c_dev *dev)
> data &= ~PUNIT_SEMAPHORE_BIT;
> if (iosf_mbi_write(BT_MBI_UNIT_PMC, MBI_REG_WRITE,
> PUNIT_SEMAPHORE, data))
> dev_err(dev->dev, "iosf failed to reset punit
> semaphore during write\n");
> +
> + pm_qos_update_request(&dev->pm_qos, PM_QOS_DEFAULT_VALUE);
> }
>
> static int baytrail_i2c_acquire(struct dw_i2c_dev *dev)
> @@ -145,6 +155,8 @@ int i2c_dw_eval_lock_support(struct dw_i2c_dev
> *dev)
> return -EPROBE_DEFER;
>
> dev_info(dev->dev, "I2C bus managed by PUNIT\n");
> + pm_qos_add_request(&dev->pm_qos, PM_QOS_CPU_DMA_LATENCY,
> + PM_QOS_DEFAULT_VALUE);
Perhaps move this to the end of the function? For sake of readability.
> dev->acquire_lock = baytrail_i2c_acquire;
> dev->release_lock = baytrail_i2c_release;
> dev->pm_runtime_disabled = true;
> diff --git a/drivers/i2c/busses/i2c-designware-core.h
> b/drivers/i2c/busses/i2c-designware-core.h
> index fb143f5..47d284c 100644
> --- a/drivers/i2c/busses/i2c-designware-core.h
> +++ b/drivers/i2c/busses/i2c-designware-core.h
> @@ -22,6 +22,7 @@
> *
> */
>
> +#include <linux/pm_qos.h>
>
> #define DW_IC_CON_MASTER 0x1
> #define DW_IC_CON_SPEED_STD 0x2
> @@ -67,6 +68,7 @@
> * @fp_lcnt: fast plus LCNT value
> * @hs_hcnt: high speed HCNT value
> * @hs_lcnt: high speed LCNT value
> + * @pm_qos: pm_qos_request used while holding a hardware lock on the
> bus
> * @acquire_lock: function to acquire a hardware lock on the bus
> * @release_lock: function to release a hardware lock on the bus
> * @pm_runtime_disabled: true if pm runtime is disabled
> @@ -114,6 +116,7 @@ struct dw_i2c_dev {
> u16 fp_lcnt;
> u16 hs_hcnt;
> u16 hs_lcnt;
> + struct pm_qos_request pm_qos;
> int (*acquire_lock)(struct dw_i2c_dev
> *dev);
> void (*release_lock)(struct dw_i2c_dev
> *dev);
> bool pm_runtime_disabled;
> diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c
> b/drivers/i2c/busses/i2c-designware-platdrv.c
> index 97a2ca1..6d72929 100644
> --- a/drivers/i2c/busses/i2c-designware-platdrv.c
> +++ b/drivers/i2c/busses/i2c-designware-platdrv.c
> @@ -291,6 +291,9 @@ static int dw_i2c_plat_remove(struct
> platform_device *pdev)
> if (!dev->pm_runtime_disabled)
> pm_runtime_disable(&pdev->dev);
>
>
> + if (dev->acquire_lock)
> + pm_qos_remove_request(&dev->pm_qos);
> +
I read your answer to v2 of this, so, taking into consideration your
notice I would recommend to provide an additional helper like
i2c_dw_down_lock_support() (I didn't find proper antonym for eval, feel
free to choose a better name) and collect the above there.
> return 0;
> }
>
--
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy
^ permalink raw reply
* Re: [PATCH v3 5/5] i2c: designware-baytrail: Add support for cherrytrail
From: Andy Shevchenko @ 2016-12-12 10:31 UTC (permalink / raw)
To: Hans de Goede, Jarkko Nikula, Wolfram Sang
Cc: Mika Westerberg, Takashi Iwai, russianneuromancer @ ya . ru,
Vincent Gerris, linux-i2c
In-Reply-To: <20161210224350.10290-5-hdegoede@redhat.com>
On Sat, 2016-12-10 at 23:43 +0100, Hans de Goede wrote:
> The cherrytrail punit has the pmic i2c bus access semaphore at a
> different register address.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> Reviewed-by: Takashi Iwai <tiwai@suse.de>
> Tested-by: Takashi Iwai <tiwai@suse.de>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
One small nit: you are using CHV abbreviation for CherryTrail, we are
consider CHT is a right one, CHV for CherryView.
> ---
> Changes in v2:
> -Adjust for accessor_flags -> flags rename
> -Add flags field to struct dw_pci_controller
> -Add get_sem_addr() helper replacing MODEL_CHERRYTRAIL flag checking
> in
> PUNIT_SEMAPHORE macro
> Changes in v3:
> -Add a gap between ACCESS_* and MODEL_* flags as reserved space for
> future ACCESS_* flags
> ---
> drivers/i2c/busses/i2c-designware-baytrail.c | 22 +++++++++++++++++
> -----
> drivers/i2c/busses/i2c-designware-core.h | 2 ++
> drivers/i2c/busses/i2c-designware-pcidrv.c | 26
> +++++++++++++++++++-------
> drivers/i2c/busses/i2c-designware-platdrv.c | 2 +-
> 4 files changed, 39 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-designware-baytrail.c
> b/drivers/i2c/busses/i2c-designware-baytrail.c
> index 997d048..456a236 100644
> --- a/drivers/i2c/busses/i2c-designware-baytrail.c
> +++ b/drivers/i2c/busses/i2c-designware-baytrail.c
> @@ -24,13 +24,23 @@
>
> #define SEMAPHORE_TIMEOUT 100
> #define PUNIT_SEMAPHORE 0x7
> +#define PUNIT_SEMAPHORE_CHV 0x10e
> #define PUNIT_SEMAPHORE_BIT BIT(0)
> #define PUNIT_SEMAPHORE_ACQUIRE BIT(1)
>
> static unsigned long acquired;
>
> +static u32 get_sem_addr(struct dw_i2c_dev *dev)
> +{
> + if (dev->flags & MODEL_CHERRYTRAIL)
> + return PUNIT_SEMAPHORE_CHV;
> + else
> + return PUNIT_SEMAPHORE;
> +}
> +
> static int get_sem(struct dw_i2c_dev *dev, u32 *sem)
> {
> + u32 addr = get_sem_addr(dev);
> u32 data;
> int ret;
>
> @@ -41,7 +51,7 @@ static int get_sem(struct dw_i2c_dev *dev, u32 *sem)
> */
> pm_qos_update_request(&dev->pm_qos, 0);
>
> - ret = iosf_mbi_read(BT_MBI_UNIT_PMC, MBI_REG_READ,
> PUNIT_SEMAPHORE, &data);
> + ret = iosf_mbi_read(BT_MBI_UNIT_PMC, MBI_REG_READ, addr,
> &data);
> if (ret) {
> dev_err(dev->dev, "iosf failed to read punit
> semaphore\n");
> return ret;
> @@ -54,15 +64,16 @@ static int get_sem(struct dw_i2c_dev *dev, u32
> *sem)
>
> static void reset_semaphore(struct dw_i2c_dev *dev)
> {
> + u32 addr = get_sem_addr(dev);
> u32 data;
>
> - if (iosf_mbi_read(BT_MBI_UNIT_PMC, MBI_REG_READ,
> PUNIT_SEMAPHORE, &data)) {
> + if (iosf_mbi_read(BT_MBI_UNIT_PMC, MBI_REG_READ, addr,
> &data)) {
> dev_err(dev->dev, "iosf failed to reset punit
> semaphore during read\n");
> return;
> }
>
> data &= ~PUNIT_SEMAPHORE_BIT;
> - if (iosf_mbi_write(BT_MBI_UNIT_PMC, MBI_REG_WRITE,
> PUNIT_SEMAPHORE, data))
> + if (iosf_mbi_write(BT_MBI_UNIT_PMC, MBI_REG_WRITE, addr,
> data))
> dev_err(dev->dev, "iosf failed to reset punit
> semaphore during write\n");
>
> pm_qos_update_request(&dev->pm_qos, PM_QOS_DEFAULT_VALUE);
> @@ -70,6 +81,7 @@ static void reset_semaphore(struct dw_i2c_dev *dev)
>
> static int baytrail_i2c_acquire(struct dw_i2c_dev *dev)
> {
> + u32 addr = get_sem_addr(dev);
> u32 sem = PUNIT_SEMAPHORE_ACQUIRE;
> int ret;
> unsigned long start, end;
> @@ -83,7 +95,7 @@ static int baytrail_i2c_acquire(struct dw_i2c_dev
> *dev)
> return 0;
>
> /* host driver writes to side band semaphore register */
> - ret = iosf_mbi_write(BT_MBI_UNIT_PMC, MBI_REG_WRITE,
> PUNIT_SEMAPHORE, sem);
> + ret = iosf_mbi_write(BT_MBI_UNIT_PMC, MBI_REG_WRITE, addr,
> sem);
> if (ret) {
> dev_err(dev->dev, "iosf punit semaphore request
> failed\n");
> return ret;
> @@ -107,7 +119,7 @@ static int baytrail_i2c_acquire(struct dw_i2c_dev
> *dev)
> dev_err(dev->dev, "punit semaphore timed out, resetting\n");
> reset_semaphore(dev);
>
> - ret = iosf_mbi_read(BT_MBI_UNIT_PMC, MBI_REG_READ,
> PUNIT_SEMAPHORE, &sem);
> + ret = iosf_mbi_read(BT_MBI_UNIT_PMC, MBI_REG_READ, addr,
> &sem);
> if (ret)
> dev_err(dev->dev, "iosf failed to read punit
> semaphore\n");
> else
> diff --git a/drivers/i2c/busses/i2c-designware-core.h
> b/drivers/i2c/busses/i2c-designware-core.h
> index 47d284c..2db3177 100644
> --- a/drivers/i2c/busses/i2c-designware-core.h
> +++ b/drivers/i2c/busses/i2c-designware-core.h
> @@ -127,6 +127,8 @@ struct dw_i2c_dev {
> #define ACCESS_16BIT 0x00000002
> #define ACCESS_INTR_MASK 0x00000004
>
> +#define MODEL_CHERRYTRAIL 0x00000100
> +
> extern int i2c_dw_init(struct dw_i2c_dev *dev);
> extern void i2c_dw_disable(struct dw_i2c_dev *dev);
> extern void i2c_dw_disable_int(struct dw_i2c_dev *dev);
> diff --git a/drivers/i2c/busses/i2c-designware-pcidrv.c
> b/drivers/i2c/busses/i2c-designware-pcidrv.c
> index 96f8230..4e53a9f 100644
> --- a/drivers/i2c/busses/i2c-designware-pcidrv.c
> +++ b/drivers/i2c/busses/i2c-designware-pcidrv.c
> @@ -45,6 +45,7 @@ enum dw_pci_ctl_id_t {
> medfield,
> merrifield,
> baytrail,
> + cherrytrail,
> haswell,
> };
>
> @@ -63,6 +64,7 @@ struct dw_pci_controller {
> u32 rx_fifo_depth;
> u32 clk_khz;
> u32 functionality;
> + u32 flags;
> struct dw_scl_sda_cfg *scl_sda_cfg;
> int (*setup)(struct pci_dev *pdev, struct dw_pci_controller
> *c);
> };
> @@ -174,6 +176,15 @@ static struct dw_pci_controller
> dw_pci_controllers[] = {
> .functionality = I2C_FUNC_10BIT_ADDR,
> .scl_sda_cfg = &hsw_config,
> },
> + [cherrytrail] = {
> + .bus_num = -1,
> + .bus_cfg = INTEL_MID_STD_CFG | DW_IC_CON_SPEED_FAST,
> + .tx_fifo_depth = 32,
> + .rx_fifo_depth = 32,
> + .functionality = I2C_FUNC_10BIT_ADDR,
> + .flags = MODEL_CHERRYTRAIL,
> + .scl_sda_cfg = &byt_config,
> + },
> };
>
> #ifdef CONFIG_PM
> @@ -241,6 +252,7 @@ static int i2c_dw_pci_probe(struct pci_dev *pdev,
> dev->base = pcim_iomap_table(pdev)[0];
> dev->dev = &pdev->dev;
> dev->irq = pdev->irq;
> + dev->flags |= controller->flags;
>
> if (controller->setup) {
> r = controller->setup(pdev, controller);
> @@ -321,13 +333,13 @@ static const struct pci_device_id
> i2_designware_pci_ids[] = {
> { PCI_VDEVICE(INTEL, 0x9c61), haswell },
> { PCI_VDEVICE(INTEL, 0x9c62), haswell },
> /* Braswell / Cherrytrail */
> - { PCI_VDEVICE(INTEL, 0x22C1), baytrail },
> - { PCI_VDEVICE(INTEL, 0x22C2), baytrail },
> - { PCI_VDEVICE(INTEL, 0x22C3), baytrail },
> - { PCI_VDEVICE(INTEL, 0x22C4), baytrail },
> - { PCI_VDEVICE(INTEL, 0x22C5), baytrail },
> - { PCI_VDEVICE(INTEL, 0x22C6), baytrail },
> - { PCI_VDEVICE(INTEL, 0x22C7), baytrail },
> + { PCI_VDEVICE(INTEL, 0x22C1), cherrytrail },
> + { PCI_VDEVICE(INTEL, 0x22C2), cherrytrail },
> + { PCI_VDEVICE(INTEL, 0x22C3), cherrytrail },
> + { PCI_VDEVICE(INTEL, 0x22C4), cherrytrail },
> + { PCI_VDEVICE(INTEL, 0x22C5), cherrytrail },
> + { PCI_VDEVICE(INTEL, 0x22C6), cherrytrail },
> + { PCI_VDEVICE(INTEL, 0x22C7), cherrytrail },
> { 0,}
> };
> MODULE_DEVICE_TABLE(pci, i2_designware_pci_ids);
> diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c
> b/drivers/i2c/busses/i2c-designware-platdrv.c
> index 6d72929..589f07e 100644
> --- a/drivers/i2c/busses/i2c-designware-platdrv.c
> +++ b/drivers/i2c/busses/i2c-designware-platdrv.c
> @@ -123,7 +123,7 @@ static const struct acpi_device_id
> dw_i2c_acpi_match[] = {
> { "INT3432", 0 },
> { "INT3433", 0 },
> { "80860F41", 0 },
> - { "808622C1", 0 },
> + { "808622C1", MODEL_CHERRYTRAIL },
> { "AMD0010", ACCESS_INTR_MASK },
> { "AMDI0010", ACCESS_INTR_MASK },
> { "AMDI0510", 0 },
--
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy
^ permalink raw reply
* Re: [PATCH v3 3/5] i2c: designware-baytrail: Only check iosf_mbi_available() for shared hosts
From: Andy Shevchenko @ 2016-12-12 10:19 UTC (permalink / raw)
To: Hans de Goede, Jarkko Nikula, Wolfram Sang
Cc: Mika Westerberg, Takashi Iwai, russianneuromancer @ ya . ru,
Vincent Gerris, linux-i2c
In-Reply-To: <20161210224350.10290-3-hdegoede@redhat.com>
On Sat, 2016-12-10 at 23:43 +0100, Hans de Goede wrote:
> If (!shared_host) simply return 0, this avoids delaying the probe if
> iosf_mbi_available() returns false when an i2c bus is not using the
> punit semaphore.
>
> Also move the if (!iosf_mbi_available()) check to above the
> dev_info, so that we do not repeat the dev_info on every probe
> until iosf_mbi_available() returns true.
>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> Changes in v2:
> -New patch in v2 of this set
> Changes in v3:
> -Use if (!shared_host) return 0, to simplify the non-shared_host path
> and to avoid nested ifs
> ---
> drivers/i2c/busses/i2c-designware-baytrail.c | 13 +++++++------
> 1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-designware-baytrail.c
> b/drivers/i2c/busses/i2c-designware-baytrail.c
> index a3f581c..cf02222 100644
> --- a/drivers/i2c/busses/i2c-designware-baytrail.c
> +++ b/drivers/i2c/busses/i2c-designware-baytrail.c
> @@ -138,15 +138,16 @@ int i2c_dw_eval_lock_support(struct dw_i2c_dev
> *dev)
> if (ACPI_FAILURE(status))
> return 0;
>
> - if (shared_host) {
> - dev_info(dev->dev, "I2C bus managed by PUNIT\n");
> - dev->acquire_lock = baytrail_i2c_acquire;
> - dev->release_lock = baytrail_i2c_release;
> - dev->pm_runtime_disabled = true;
> - }
> + if (!shared_host)
> + return 0;
>
> if (!iosf_mbi_available())
> return -EPROBE_DEFER;
>
> + dev_info(dev->dev, "I2C bus managed by PUNIT\n");
> + dev->acquire_lock = baytrail_i2c_acquire;
> + dev->release_lock = baytrail_i2c_release;
> + dev->pm_runtime_disabled = true;
> +
> return 0;
> }
--
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy
^ permalink raw reply
* [PATCH V3] i2c: designware: fix wrong tx/rx fifo for ACPI
From: Tin Huynh @ 2016-12-12 8:36 UTC (permalink / raw)
To: Jarkko Nikula, Andy Shevchenko, Mika Westerberg, Wolfram Sang
Cc: linux-i2c, linux-kernel, linux-acpi, Loc Ho, Thang Nguyen,
Phong Vo, patches, Tin Huynh
ACPI always sets txfifo and rxfifo to 32. This configuration will
cause problem if the IP core supports a fifo size of less than 32.
The driver should read the fifo size from the IP and select the
smaller one of the two.
Signed-off-by: Tin Huynh <tnhuynh@apm.com>
---
drivers/i2c/busses/i2c-designware-platdrv.c | 26 ++++++++++++++++++++------
1 files changed, 20 insertions(+), 6 deletions(-)
Change from V2:
-Add a helper function to set fifo size.
Change from V1:
-Revert the default 32 for fifo, read parameter from IP core
and pick the smaller one of the two.
-Correct the title to describe new approach.
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index 0b42a12..665f491 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -150,6 +150,24 @@ static int i2c_dw_plat_prepare_clk(struct dw_i2c_dev *i_dev, bool prepare)
return 0;
}
+static void dw_i2c_set_fifo_size(struct dw_i2c_dev *dev)
+{
+ u32 param1, tx_fifo_depth, rx_fifo_depth;
+
+ param1 = i2c_dw_read_comp_param(dev);
+ tx_fifo_depth = ((param1 >> 16) & 0xff) + 1;
+ rx_fifo_depth = ((param1 >> 8) & 0xff) + 1;
+ if (!dev->tx_fifo_depth) {
+ dev->tx_fifo_depth = tx_fifo_depth;
+ dev->rx_fifo_depth = rx_fifo_depth;
+ } else if (tx_fifo_depth) {
+ dev->tx_fifo_depth = min_t(u32, dev->tx_fifo_depth,
+ tx_fifo_depth);
+ dev->rx_fifo_depth = min_t(u32, dev->rx_fifo_depth,
+ rx_fifo_depth);
+ }
+}
+
static int dw_i2c_plat_probe(struct platform_device *pdev)
{
struct dw_i2c_platform_data *pdata = dev_get_platdata(&pdev->dev);
@@ -246,13 +264,9 @@ static int dw_i2c_plat_probe(struct platform_device *pdev)
1000000);
}
- if (!dev->tx_fifo_depth) {
- u32 param1 = i2c_dw_read_comp_param(dev);
-
- dev->tx_fifo_depth = ((param1 >> 16) & 0xff) + 1;
- dev->rx_fifo_depth = ((param1 >> 8) & 0xff) + 1;
+ if (!dev->tx_fifo_depth)
dev->adapter.nr = pdev->id;
- }
+ dw_i2c_set_fifo_size(dev);
adap = &dev->adapter;
adap->owner = THIS_MODULE;
--
1.7.1
^ permalink raw reply related
* Re: [PATCH v5 2/5] i2c: Add STM32F4 I2C driver
From: M'boumba Cedric Madianga @ 2016-12-12 8:34 UTC (permalink / raw)
To: Wolfram Sang
Cc: Rob Herring, Maxime Coquelin, Alexandre Torgue, Linus Walleij,
Patrice Chotard, linux-I+IVW8TIWO2tmTQ+vhA3Yw,
linux-i2c-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161211214232.GA2552@katana>
Hi Wolfram,
Thanks for your comments.
I will fix all of them + build errors reported by build-bot in the V6.
Best regards,
Cedric Madianga
2016-12-11 22:42 GMT+01:00 Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org>:
> Hi,
>
>> +config I2C_STM32F4
>> + tristate "STMicroelectronics STM32F4 I2C support"
>> + depends on ARCH_STM32 || COMPILE_TEST
>
> Double space.
>
>> +#define STM32F4_I2C_MIN_FREQ 2
>> +#define STM32F4_I2C_MAX_FREQ 42
>
> Those two must be unsigned to fix the build error (e.g. 2U) reported by
> build-bot.
>
> Also, I get the following build warnings:
>
> CC drivers/i2c/busses/i2c-stm32f4.o
> drivers/i2c/busses/i2c-stm32f4.c: In function ‘stm32f4_i2c_handle_rx_addr’:
> drivers/i2c/busses/i2c-stm32f4.c:445:6: warning: variable ‘sr2’ set but not used [-Wunused-but-set-variable]
> u32 sr2;
> ^~~
> drivers/i2c/busses/i2c-stm32f4.c: In function ‘stm32f4_i2c_isr_event’:
> drivers/i2c/busses/i2c-stm32f4.c:496:41: warning: variable ‘sr2’ set but not used [-Wunused-but-set-variable]
> u32 real_status, possible_status, ien, sr2;
>
> I assume those are reads to clear the register, so we really don't need
> to save the value in a variable.
>
> Rest is looking good.
>
> Thanks,
>
> Wolfram
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v5 1/2] i2c: aspeed: added driver for Aspeed I2C
From: Wolfram Sang @ 2016-12-11 22:26 UTC (permalink / raw)
To: Brendan Higgins
Cc: vz, clg, mouse, robh+dt, mark.rutland, linux-i2c, devicetree,
joel, openbmc
In-Reply-To: <1480467618-7497-2-git-send-email-brendanhiggins@google.com>
[-- Attachment #1: Type: text/plain, Size: 782 bytes --]
On Tue, Nov 29, 2016 at 05:00:17PM -0800, Brendan Higgins wrote:
> Added initial master and slave support for Aspeed I2C controller.
> Supports fourteen busses present in ast24xx and ast25xx BMC SoCs by
> Aspeed.
>
> Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
BTW first the bindings patch please, then the driver.
And one seperate question I just stumbled over:
> + /* Switch from master mode to slave mode. */
> + func_ctrl_reg_val = aspeed_i2c_read(bus, ASPEED_I2C_FUN_CTRL_REG);
> + func_ctrl_reg_val &= ~ASPEED_I2CD_MASTER_EN;
> + func_ctrl_reg_val |= ASPEED_I2CD_SLAVE_EN;
> + aspeed_i2c_write(bus, func_ctrl_reg_val, ASPEED_I2C_FUN_CTRL_REG);
Can't the hardware work both as master and slave on the same bus?
Regards,
Wolfram
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH] i2c: core: don't try to OF populate DDC i2c buses
From: Wolfram Sang @ 2016-12-11 22:16 UTC (permalink / raw)
To: Vladimir Zapolskiy; +Cc: Lucas Stach, linux-i2c, kernel, patchwork-lst
In-Reply-To: <f50333f5-906b-fe9a-3e5b-e74522472d06@mleia.com>
[-- Attachment #1: Type: text/plain, Size: 955 bytes --]
Lucas,
> Shallow review of DTS files and dpaux drivers let me say that the change
> below has no regressions (the change is untested):
>
> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> index 3e6fe82..f91ade1 100644
> --- a/drivers/gpu/drm/drm_dp_helper.c
> +++ b/drivers/gpu/drm/drm_dp_helper.c
> @@ -1020,7 +1020,6 @@ int drm_dp_aux_register(struct drm_dp_aux *aux)
> aux->ddc.class = I2C_CLASS_DDC;
> aux->ddc.owner = THIS_MODULE;
> aux->ddc.dev.parent = aux->dev;
> - aux->ddc.dev.of_node = aux->dev->of_node;
>
> strlcpy(aux->ddc.name, aux->name ? aux->name : dev_name(aux->dev),
> sizeof(aux->ddc.name));
Is this an acceptable approach? It makes sense to me from an I2C PoV.
I have to say that I second Vladimir's arguments. There shouldn't be any
other I2C devices on DDC, but surely there is somebody somewhere hacking
this bus to do something.
Regards,
Wolfram
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [Patch V1] i2c: fsl-lpi2c: read lpi2c fifo size in probe()
From: Wolfram Sang @ 2016-12-11 22:07 UTC (permalink / raw)
To: Gao Pan; +Cc: wsa-dev, cmo, robh, vz, linux-i2c, frank.li, fugang.duan
In-Reply-To: <1480649896-5222-1-git-send-email-pandy.gao@nxp.com>
[-- Attachment #1: Type: text/plain, Size: 316 bytes --]
On Fri, Dec 02, 2016 at 11:38:16AM +0800, Gao Pan wrote:
> The lpi2c fifo size is a read only parameter resides Parameter
> Register. It's better to read lpi2c tx/rx fifo size in probe()
> other than just define a macro for it.
>
> Signed-off-by: Gao Pan <pandy.gao@nxp.com>
Applied to for-next, thanks!
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH 4/4] i2c: octeon: thunderx: Add I2C_CLASS_HWMON
From: Wolfram Sang @ 2016-12-11 22:04 UTC (permalink / raw)
To: Jan Glauber
Cc: Wolfram Sang, Paul Burton, Steven J . Hill, linux-i2c, linux-mips,
David Daney
In-Reply-To: <20161209093158.3161-5-jglauber@cavium.com>
[-- Attachment #1: Type: text/plain, Size: 593 bytes --]
On Fri, Dec 09, 2016 at 10:31:58AM +0100, Jan Glauber wrote:
> It was reported that ipmi_ssif fails to create the
> ipmi device on some systems if the adapter class is not containing
> I2C_CLASS_HWMON. Fix it by setting the class.
>
> Reported-by: Vadim Lomovtsev <Vadim.Lomovtsev@caviumnetworks.com>
> Signed-off-by: Jan Glauber <jglauber@cavium.com>
The intention of adapter classes is to *limit* probing to a certain
class of devices. If a class is needed to *enable* probing, then
something there looks wrong. From the details given, this must be solved
elsewhere I'd say.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH 3/4] i2c: octeon: thunderx: Limit register access retries
From: Wolfram Sang @ 2016-12-11 22:01 UTC (permalink / raw)
To: Jan Glauber
Cc: Wolfram Sang, Paul Burton, Steven J . Hill, linux-i2c, linux-mips,
David Daney
In-Reply-To: <20161209093158.3161-4-jglauber@cavium.com>
[-- Attachment #1: Type: text/plain, Size: 424 bytes --]
On Fri, Dec 09, 2016 at 10:31:57AM +0100, Jan Glauber wrote:
> Do not infinitely retry register readq and writeq operations
> in order to not lock up the CPU in case the TWSI gets stuck.
>
> Return -EIO in case of a failed data read. For all other
> cases just return so subsequent operations will fail
> and trigger the recovery.
>
> Signed-off-by: Jan Glauber <jglauber@cavium.com>
I can't apply this one?
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH 2/4] i2c: octeon: thunderx: Remove double-check after interrupt
From: Wolfram Sang @ 2016-12-11 22:01 UTC (permalink / raw)
To: Jan Glauber
Cc: Wolfram Sang, Paul Burton, Steven J . Hill, linux-i2c, linux-mips,
David Daney
In-Reply-To: <20161209093158.3161-3-jglauber@cavium.com>
[-- Attachment #1: Type: text/plain, Size: 501 bytes --]
On Fri, Dec 09, 2016 at 10:31:56AM +0100, Jan Glauber wrote:
> Commit 1bb1ff3e7c74 ("i2c: octeon: Improve performance if interrupt is
> early") added a double-check around the wait_event_timeout() condition.
> The performance problem that this commit tried to work-around
> could not be reproduced. It also makes the wait condition more
> complicated then it should be. Therefore remove the double-check.
>
> Signed-off-by: Jan Glauber <jglauber@cavium.com>
Applied to for-next, thanks!
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH 1/4] i2c: octeon: thunderx: TWSI software reset in recovery
From: Wolfram Sang @ 2016-12-11 22:01 UTC (permalink / raw)
To: Jan Glauber
Cc: Wolfram Sang, Paul Burton, Steven J . Hill, linux-i2c, linux-mips,
David Daney
In-Reply-To: <20161209093158.3161-2-jglauber@cavium.com>
[-- Attachment #1: Type: text/plain, Size: 571 bytes --]
On Fri, Dec 09, 2016 at 10:31:55AM +0100, Jan Glauber wrote:
> I've seen i2c recovery reporting long loops of:
>
> [ 1035.887818] i2c i2c-4: SCL is stuck low, exit recovery
> [ 1037.999748] i2c i2c-4: SCL is stuck low, exit recovery
> [ 1040.111694] i2c i2c-4: SCL is stuck low, exit recovery
> ...
>
> Add a TWSI software reset which clears the status and
> STA,STP,IFLG in SW_TWSI_EOP_TWSI_CTL.
>
> With this the recovery works fine and above message is not seen.
>
> Signed-off-by: Jan Glauber <jglauber@cavium.com>
Applied to for-next, thanks!
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH] i2c: cadence: Allow Cadence I2C to be selected for Cadence Xtensa CPUs
From: Wolfram Sang @ 2016-12-11 21:57 UTC (permalink / raw)
To: Jan Kotas; +Cc: linux-i2c, linux-kernel
In-Reply-To: <1481190478-16718-1-git-send-email-jank@cadence.com>
[-- Attachment #1: Type: text/plain, Size: 248 bytes --]
On Thu, Dec 08, 2016 at 09:47:58AM +0000, Jan Kotas wrote:
> This patch allows Cadence I2C controller to be selected in systems using Cadence Xtensa processors.
>
> Signed-off-by: Jan Kotas <jank@cadence.com>
Applied to for-next, thanks!
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH 2/2] i2c: sh_mobile: Add per-Generation fallback bindings
From: Wolfram Sang @ 2016-12-11 21:53 UTC (permalink / raw)
To: Simon Horman
Cc: Wolfram Sang, Magnus Damm, linux-i2c, linux-renesas-soc,
devicetree
In-Reply-To: <1481107193-32502-1-git-send-email-horms+renesas@verge.net.au>
[-- Attachment #1: Type: text/plain, Size: 390 bytes --]
On Wed, Dec 07, 2016 at 11:39:53AM +0100, Simon Horman wrote:
> Add per-Generation fallback bindings for R-Car SoCs.
>
> This is in keeping with the compatibility string scheme is being adopted
> for drivers for Renesas SoCs.
>
> Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
I took the liberty to merge patch 1/2 into this one. Then, applied to
for-next, thanks!
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH v4 i2c/for-next] i2c: rcar: Add per-Generation fallback bindings
From: Wolfram Sang @ 2016-12-11 21:49 UTC (permalink / raw)
To: Simon Horman
Cc: Wolfram Sang, Magnus Damm, linux-i2c, linux-renesas-soc,
Rob Herring, devicetree
In-Reply-To: <1481040088-22596-1-git-send-email-horms+renesas@verge.net.au>
[-- Attachment #1: Type: text/plain, Size: 1209 bytes --]
On Tue, Dec 06, 2016 at 05:01:28PM +0100, Simon Horman wrote:
> In the case of Renesas R-Car hardware we know that there are generations of
> SoCs, e.g. Gen 2 and Gen 3. But beyond that it's not clear what the
> relationship between IP blocks might be. For example, I believe that
> r8a7790 is older than r8a7791 but that doesn't imply that the latter is a
> descendant of the former or vice versa.
>
> We can, however, by examining the documentation and behaviour of the
> hardware at run-time observe that the current driver implementation appears
> to be compatible with the IP blocks on SoCs within a given generation.
>
> For the above reasons and convenience when enabling new SoCs a
> per-generation fallback compatibility string scheme is being adopted for
> drivers for Renesas SoCs.
>
> Also:
> * Deprecate renesas,i2c-rcar. It seems poorly named as it is only
> compatible with R-Car Gen 1. It also appears unused in mainline.
> * Add some text to describe per-SoC bindings
>
> Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Changed '//' comment to '/*' comment and applied to for-next, thanks!
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH v5 2/5] i2c: Add STM32F4 I2C driver
From: Wolfram Sang @ 2016-12-11 21:42 UTC (permalink / raw)
To: M'boumba Cedric Madianga
Cc: robh+dt, mcoquelin.stm32, alexandre.torgue, linus.walleij,
patrice.chotard, linux, linux-i2c, devicetree, linux-arm-kernel,
linux-kernel
In-Reply-To: <1481185563-8735-3-git-send-email-cedric.madianga@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1016 bytes --]
Hi,
> +config I2C_STM32F4
> + tristate "STMicroelectronics STM32F4 I2C support"
> + depends on ARCH_STM32 || COMPILE_TEST
Double space.
> +#define STM32F4_I2C_MIN_FREQ 2
> +#define STM32F4_I2C_MAX_FREQ 42
Those two must be unsigned to fix the build error (e.g. 2U) reported by
build-bot.
Also, I get the following build warnings:
CC drivers/i2c/busses/i2c-stm32f4.o
drivers/i2c/busses/i2c-stm32f4.c: In function ‘stm32f4_i2c_handle_rx_addr’:
drivers/i2c/busses/i2c-stm32f4.c:445:6: warning: variable ‘sr2’ set but not used [-Wunused-but-set-variable]
u32 sr2;
^~~
drivers/i2c/busses/i2c-stm32f4.c: In function ‘stm32f4_i2c_isr_event’:
drivers/i2c/busses/i2c-stm32f4.c:496:41: warning: variable ‘sr2’ set but not used [-Wunused-but-set-variable]
u32 real_status, possible_status, ien, sr2;
I assume those are reads to clear the register, so we really don't need
to save the value in a variable.
Rest is looking good.
Thanks,
Wolfram
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH v4 4/5] i2c: designware: Add slave mode as separated driver
From: kbuild test robot @ 2016-12-10 23:44 UTC (permalink / raw)
Cc: kbuild-all, wsa, robh+dt, mark.rutland, jarkko.nikula,
andriy.shevchenko, mika.westerberg, linux-i2c, devicetree,
linux-kernel, Luis.Oliveira, Ramiro.Oliveira, Joao.Pinto,
CARLOS.PALMINHA
In-Reply-To: <a7ca5014ad1c3f4905349a02ebe5294fe64c318e.1481131072.git.lolivei@synopsys.com>
[-- Attachment #1: Type: text/plain, Size: 6820 bytes --]
Hi Luis,
[auto build test ERROR on wsa/i2c/for-next]
[also build test ERROR on next-20161209]
[cannot apply to v4.9-rc8]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Luis-Oliveira/i2c-designware-Refactoring-of-the-i2c-designware/20161208-044045
base: https://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git i2c/for-next
config: i386-randconfig-c0-12110449 (attached as .config)
compiler: gcc-4.9 (Debian 4.9.4-2) 4.9.4
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All errors (new ones prefixed by >>):
drivers/i2c/busses/i2c-designware-slave.c: In function 'i2c_dw_irq_handler_slave':
>> drivers/i2c/busses/i2c-designware-slave.c:294:3: error: implicit declaration of function 'i2c_slave_event' [-Werror=implicit-function-declaration]
i2c_slave_event(dev->slave, I2C_SLAVE_WRITE_REQUESTED, &val);
^
>> drivers/i2c/busses/i2c-designware-slave.c:294:31: error: 'I2C_SLAVE_WRITE_REQUESTED' undeclared (first use in this function)
i2c_slave_event(dev->slave, I2C_SLAVE_WRITE_REQUESTED, &val);
^
drivers/i2c/busses/i2c-designware-slave.c:294:31: note: each undeclared identifier is reported only once for each function it appears in
In file included from include/linux/err.h:4:0,
from drivers/i2c/busses/i2c-designware-slave.c:26:
>> drivers/i2c/busses/i2c-designware-slave.c:301:6: error: 'I2C_SLAVE_WRITE_RECEIVED' undeclared (first use in this function)
I2C_SLAVE_WRITE_RECEIVED, &val)) {
^
include/linux/compiler.h:149:30: note: in definition of macro '__trace_if'
if (__builtin_constant_p(!!(cond)) ? !!(cond) : \
^
drivers/i2c/busses/i2c-designware-slave.c:300:5: note: in expansion of macro 'if'
if (!i2c_slave_event(dev->slave,
^
>> drivers/i2c/busses/i2c-designware-slave.c:313:7: error: 'I2C_SLAVE_READ_REQUESTED' undeclared (first use in this function)
I2C_SLAVE_READ_REQUESTED, &val))
^
include/linux/compiler.h:149:30: note: in definition of macro '__trace_if'
if (__builtin_constant_p(!!(cond)) ? !!(cond) : \
^
drivers/i2c/busses/i2c-designware-slave.c:312:4: note: in expansion of macro 'if'
if (!i2c_slave_event(dev->slave,
^
>> drivers/i2c/busses/i2c-designware-slave.c:319:36: error: 'I2C_SLAVE_READ_PROCESSED' undeclared (first use in this function)
if (!i2c_slave_event(dev->slave, I2C_SLAVE_READ_PROCESSED,
^
include/linux/compiler.h:149:30: note: in definition of macro '__trace_if'
if (__builtin_constant_p(!!(cond)) ? !!(cond) : \
^
drivers/i2c/busses/i2c-designware-slave.c:319:3: note: in expansion of macro 'if'
if (!i2c_slave_event(dev->slave, I2C_SLAVE_READ_PROCESSED,
^
>> drivers/i2c/busses/i2c-designware-slave.c:323:31: error: 'I2C_SLAVE_STOP' undeclared (first use in this function)
i2c_slave_event(dev->slave, I2C_SLAVE_STOP, &val);
^
drivers/i2c/busses/i2c-designware-slave.c: At top level:
>> drivers/i2c/busses/i2c-designware-slave.c:358:2: error: unknown field 'reg_slave' specified in initializer
.reg_slave = i2c_dw_reg_slave,
^
drivers/i2c/busses/i2c-designware-slave.c:358:2: warning: excess elements in struct initializer
drivers/i2c/busses/i2c-designware-slave.c:358:2: warning: (near initialization for 'i2c_dw_algo')
>> drivers/i2c/busses/i2c-designware-slave.c:359:2: error: unknown field 'unreg_slave' specified in initializer
.unreg_slave = i2c_dw_unreg_slave,
^
drivers/i2c/busses/i2c-designware-slave.c:359:2: warning: excess elements in struct initializer
drivers/i2c/busses/i2c-designware-slave.c:359:2: warning: (near initialization for 'i2c_dw_algo')
cc1: some warnings being treated as errors
vim +/i2c_slave_event +294 drivers/i2c/busses/i2c-designware-slave.c
288 dw_readl(dev, DW_IC_CLR_START_DET);
289 if (stat & DW_IC_INTR_ACTIVITY)
290 dw_readl(dev, DW_IC_CLR_ACTIVITY);
291 if (stat & DW_IC_INTR_RX_OVER)
292 dw_readl(dev, DW_IC_CLR_RX_OVER);
293 if ((stat & DW_IC_INTR_RX_FULL) && (stat & DW_IC_INTR_STOP_DET))
> 294 i2c_slave_event(dev->slave, I2C_SLAVE_WRITE_REQUESTED, &val);
295
296 if (slave_activity) {
297 if (stat & DW_IC_INTR_RD_REQ) {
298 if (stat & DW_IC_INTR_RX_FULL) {
299 val = dw_readl(dev, DW_IC_DATA_CMD);
300 if (!i2c_slave_event(dev->slave,
> 301 I2C_SLAVE_WRITE_RECEIVED, &val)) {
302 dev_dbg(dev->dev, "Byte %X acked!",
303 val);
304 }
305 dw_readl(dev, DW_IC_CLR_RD_REQ);
306 stat = i2c_dw_read_clear_intrbits_slave(dev);
307 } else {
308 dw_readl(dev, DW_IC_CLR_RD_REQ);
309 dw_readl(dev, DW_IC_CLR_RX_UNDER);
310 stat = i2c_dw_read_clear_intrbits_slave(dev);
311 }
312 if (!i2c_slave_event(dev->slave,
> 313 I2C_SLAVE_READ_REQUESTED, &val))
314 dw_writel(dev, val, DW_IC_DATA_CMD);
315 }
316 }
317
318 if (stat & DW_IC_INTR_RX_DONE) {
> 319 if (!i2c_slave_event(dev->slave, I2C_SLAVE_READ_PROCESSED,
320 &val))
321 dw_readl(dev, DW_IC_CLR_RX_DONE);
322
> 323 i2c_slave_event(dev->slave, I2C_SLAVE_STOP, &val);
324 stat = i2c_dw_read_clear_intrbits_slave(dev);
325 return true;
326 }
327
328 if (stat & DW_IC_INTR_RX_FULL) {
329 val = dw_readl(dev, DW_IC_DATA_CMD);
330 if (!i2c_slave_event(dev->slave, I2C_SLAVE_WRITE_RECEIVED,
331 &val))
332 dev_dbg(dev->dev, "Byte %X acked!", val);
333 } else {
334 i2c_slave_event(dev->slave, I2C_SLAVE_STOP, &val);
335 stat = i2c_dw_read_clear_intrbits_slave(dev);
336 }
337
338 if (stat & DW_IC_INTR_TX_OVER)
339 dw_readl(dev, DW_IC_CLR_TX_OVER);
340
341 return true;
342 }
343
344 static irqreturn_t i2c_dw_isr_slave(int this_irq, void *dev_id)
345 {
346 struct dw_i2c_dev *dev = dev_id;
347
348 i2c_dw_read_clear_intrbits_slave(dev);
349 if (!i2c_dw_irq_handler_slave(dev))
350 return IRQ_NONE;
351
352 complete(&dev->cmd_complete);
353 return IRQ_HANDLED;
354 }
355
356 static struct i2c_algorithm i2c_dw_algo = {
357 .functionality = i2c_dw_func,
> 358 .reg_slave = i2c_dw_reg_slave,
> 359 .unreg_slave = i2c_dw_unreg_slave,
360 };
361
362 void i2c_dw_disable_slave(struct dw_i2c_dev *dev)
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 31719 bytes --]
^ permalink raw reply
* [PATCH v3 5/5] i2c: designware-baytrail: Add support for cherrytrail
From: Hans de Goede @ 2016-12-10 22:43 UTC (permalink / raw)
To: Jarkko Nikula, Wolfram Sang
Cc: Andy Shevchenko, Mika Westerberg, Takashi Iwai,
russianneuromancer @ ya . ru, Vincent Gerris, linux-i2c,
Hans de Goede
In-Reply-To: <20161210224350.10290-1-hdegoede@redhat.com>
The cherrytrail punit has the pmic i2c bus access semaphore at a
different register address.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Reviewed-by: Takashi Iwai <tiwai@suse.de>
Tested-by: Takashi Iwai <tiwai@suse.de>
---
Changes in v2:
-Adjust for accessor_flags -> flags rename
-Add flags field to struct dw_pci_controller
-Add get_sem_addr() helper replacing MODEL_CHERRYTRAIL flag checking in
PUNIT_SEMAPHORE macro
Changes in v3:
-Add a gap between ACCESS_* and MODEL_* flags as reserved space for
future ACCESS_* flags
---
drivers/i2c/busses/i2c-designware-baytrail.c | 22 +++++++++++++++++-----
drivers/i2c/busses/i2c-designware-core.h | 2 ++
drivers/i2c/busses/i2c-designware-pcidrv.c | 26 +++++++++++++++++++-------
drivers/i2c/busses/i2c-designware-platdrv.c | 2 +-
4 files changed, 39 insertions(+), 13 deletions(-)
diff --git a/drivers/i2c/busses/i2c-designware-baytrail.c b/drivers/i2c/busses/i2c-designware-baytrail.c
index 997d048..456a236 100644
--- a/drivers/i2c/busses/i2c-designware-baytrail.c
+++ b/drivers/i2c/busses/i2c-designware-baytrail.c
@@ -24,13 +24,23 @@
#define SEMAPHORE_TIMEOUT 100
#define PUNIT_SEMAPHORE 0x7
+#define PUNIT_SEMAPHORE_CHV 0x10e
#define PUNIT_SEMAPHORE_BIT BIT(0)
#define PUNIT_SEMAPHORE_ACQUIRE BIT(1)
static unsigned long acquired;
+static u32 get_sem_addr(struct dw_i2c_dev *dev)
+{
+ if (dev->flags & MODEL_CHERRYTRAIL)
+ return PUNIT_SEMAPHORE_CHV;
+ else
+ return PUNIT_SEMAPHORE;
+}
+
static int get_sem(struct dw_i2c_dev *dev, u32 *sem)
{
+ u32 addr = get_sem_addr(dev);
u32 data;
int ret;
@@ -41,7 +51,7 @@ static int get_sem(struct dw_i2c_dev *dev, u32 *sem)
*/
pm_qos_update_request(&dev->pm_qos, 0);
- ret = iosf_mbi_read(BT_MBI_UNIT_PMC, MBI_REG_READ, PUNIT_SEMAPHORE, &data);
+ ret = iosf_mbi_read(BT_MBI_UNIT_PMC, MBI_REG_READ, addr, &data);
if (ret) {
dev_err(dev->dev, "iosf failed to read punit semaphore\n");
return ret;
@@ -54,15 +64,16 @@ static int get_sem(struct dw_i2c_dev *dev, u32 *sem)
static void reset_semaphore(struct dw_i2c_dev *dev)
{
+ u32 addr = get_sem_addr(dev);
u32 data;
- if (iosf_mbi_read(BT_MBI_UNIT_PMC, MBI_REG_READ, PUNIT_SEMAPHORE, &data)) {
+ if (iosf_mbi_read(BT_MBI_UNIT_PMC, MBI_REG_READ, addr, &data)) {
dev_err(dev->dev, "iosf failed to reset punit semaphore during read\n");
return;
}
data &= ~PUNIT_SEMAPHORE_BIT;
- if (iosf_mbi_write(BT_MBI_UNIT_PMC, MBI_REG_WRITE, PUNIT_SEMAPHORE, data))
+ if (iosf_mbi_write(BT_MBI_UNIT_PMC, MBI_REG_WRITE, addr, data))
dev_err(dev->dev, "iosf failed to reset punit semaphore during write\n");
pm_qos_update_request(&dev->pm_qos, PM_QOS_DEFAULT_VALUE);
@@ -70,6 +81,7 @@ static void reset_semaphore(struct dw_i2c_dev *dev)
static int baytrail_i2c_acquire(struct dw_i2c_dev *dev)
{
+ u32 addr = get_sem_addr(dev);
u32 sem = PUNIT_SEMAPHORE_ACQUIRE;
int ret;
unsigned long start, end;
@@ -83,7 +95,7 @@ static int baytrail_i2c_acquire(struct dw_i2c_dev *dev)
return 0;
/* host driver writes to side band semaphore register */
- ret = iosf_mbi_write(BT_MBI_UNIT_PMC, MBI_REG_WRITE, PUNIT_SEMAPHORE, sem);
+ ret = iosf_mbi_write(BT_MBI_UNIT_PMC, MBI_REG_WRITE, addr, sem);
if (ret) {
dev_err(dev->dev, "iosf punit semaphore request failed\n");
return ret;
@@ -107,7 +119,7 @@ static int baytrail_i2c_acquire(struct dw_i2c_dev *dev)
dev_err(dev->dev, "punit semaphore timed out, resetting\n");
reset_semaphore(dev);
- ret = iosf_mbi_read(BT_MBI_UNIT_PMC, MBI_REG_READ, PUNIT_SEMAPHORE, &sem);
+ ret = iosf_mbi_read(BT_MBI_UNIT_PMC, MBI_REG_READ, addr, &sem);
if (ret)
dev_err(dev->dev, "iosf failed to read punit semaphore\n");
else
diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
index 47d284c..2db3177 100644
--- a/drivers/i2c/busses/i2c-designware-core.h
+++ b/drivers/i2c/busses/i2c-designware-core.h
@@ -127,6 +127,8 @@ struct dw_i2c_dev {
#define ACCESS_16BIT 0x00000002
#define ACCESS_INTR_MASK 0x00000004
+#define MODEL_CHERRYTRAIL 0x00000100
+
extern int i2c_dw_init(struct dw_i2c_dev *dev);
extern void i2c_dw_disable(struct dw_i2c_dev *dev);
extern void i2c_dw_disable_int(struct dw_i2c_dev *dev);
diff --git a/drivers/i2c/busses/i2c-designware-pcidrv.c b/drivers/i2c/busses/i2c-designware-pcidrv.c
index 96f8230..4e53a9f 100644
--- a/drivers/i2c/busses/i2c-designware-pcidrv.c
+++ b/drivers/i2c/busses/i2c-designware-pcidrv.c
@@ -45,6 +45,7 @@ enum dw_pci_ctl_id_t {
medfield,
merrifield,
baytrail,
+ cherrytrail,
haswell,
};
@@ -63,6 +64,7 @@ struct dw_pci_controller {
u32 rx_fifo_depth;
u32 clk_khz;
u32 functionality;
+ u32 flags;
struct dw_scl_sda_cfg *scl_sda_cfg;
int (*setup)(struct pci_dev *pdev, struct dw_pci_controller *c);
};
@@ -174,6 +176,15 @@ static struct dw_pci_controller dw_pci_controllers[] = {
.functionality = I2C_FUNC_10BIT_ADDR,
.scl_sda_cfg = &hsw_config,
},
+ [cherrytrail] = {
+ .bus_num = -1,
+ .bus_cfg = INTEL_MID_STD_CFG | DW_IC_CON_SPEED_FAST,
+ .tx_fifo_depth = 32,
+ .rx_fifo_depth = 32,
+ .functionality = I2C_FUNC_10BIT_ADDR,
+ .flags = MODEL_CHERRYTRAIL,
+ .scl_sda_cfg = &byt_config,
+ },
};
#ifdef CONFIG_PM
@@ -241,6 +252,7 @@ static int i2c_dw_pci_probe(struct pci_dev *pdev,
dev->base = pcim_iomap_table(pdev)[0];
dev->dev = &pdev->dev;
dev->irq = pdev->irq;
+ dev->flags |= controller->flags;
if (controller->setup) {
r = controller->setup(pdev, controller);
@@ -321,13 +333,13 @@ static const struct pci_device_id i2_designware_pci_ids[] = {
{ PCI_VDEVICE(INTEL, 0x9c61), haswell },
{ PCI_VDEVICE(INTEL, 0x9c62), haswell },
/* Braswell / Cherrytrail */
- { PCI_VDEVICE(INTEL, 0x22C1), baytrail },
- { PCI_VDEVICE(INTEL, 0x22C2), baytrail },
- { PCI_VDEVICE(INTEL, 0x22C3), baytrail },
- { PCI_VDEVICE(INTEL, 0x22C4), baytrail },
- { PCI_VDEVICE(INTEL, 0x22C5), baytrail },
- { PCI_VDEVICE(INTEL, 0x22C6), baytrail },
- { PCI_VDEVICE(INTEL, 0x22C7), baytrail },
+ { PCI_VDEVICE(INTEL, 0x22C1), cherrytrail },
+ { PCI_VDEVICE(INTEL, 0x22C2), cherrytrail },
+ { PCI_VDEVICE(INTEL, 0x22C3), cherrytrail },
+ { PCI_VDEVICE(INTEL, 0x22C4), cherrytrail },
+ { PCI_VDEVICE(INTEL, 0x22C5), cherrytrail },
+ { PCI_VDEVICE(INTEL, 0x22C6), cherrytrail },
+ { PCI_VDEVICE(INTEL, 0x22C7), cherrytrail },
{ 0,}
};
MODULE_DEVICE_TABLE(pci, i2_designware_pci_ids);
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index 6d72929..589f07e 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -123,7 +123,7 @@ static const struct acpi_device_id dw_i2c_acpi_match[] = {
{ "INT3432", 0 },
{ "INT3433", 0 },
{ "80860F41", 0 },
- { "808622C1", 0 },
+ { "808622C1", MODEL_CHERRYTRAIL },
{ "AMD0010", ACCESS_INTR_MASK },
{ "AMDI0010", ACCESS_INTR_MASK },
{ "AMDI0510", 0 },
--
2.9.3
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox