LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Hans de Goede <hdegoede@redhat.com>
To: Daniel Scally <djrscally@gmail.com>,
	Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: "Rafael J . Wysocki" <rjw@rjwysocki.net>,
	Mark Gross <markgross@kernel.org>,
	Andy Shevchenko <andy@infradead.org>,
	Wolfram Sang <wsa@the-dreams.de>,
	Mika Westerberg <mika.westerberg@linux.intel.com>,
	Mauro Carvalho Chehab <mchehab@kernel.org>,
	Liam Girdwood <lgirdwood@gmail.com>,
	Mark Brown <broonie@kernel.org>,
	Michael Turquette <mturquette@baylibre.com>,
	Stephen Boyd <sboyd@kernel.org>, Len Brown <lenb@kernel.org>,
	linux-acpi@vger.kernel.org, platform-driver-x86@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-i2c@vger.kernel.org,
	Sakari Ailus <sakari.ailus@linux.intel.com>,
	Kate Hsuan <hpa@redhat.com>,
	linux-media@vger.kernel.org, linux-clk@vger.kernel.org,
	Andy Shevchenko <andy.shevchenko@gmail.com>
Subject: Re: [PATCH v6 07/15] platform/x86: int3472: Enable I2c daisy chain
Date: Fri, 26 Nov 2021 12:45:59 +0100	[thread overview]
Message-ID: <2fd5400e-e587-54d2-1071-ad8df49a8a68@redhat.com> (raw)
In-Reply-To: <4ab5efa7-65b0-009c-293a-d7a49776e78d@gmail.com>

Hi,

On 11/26/21 12:39, Daniel Scally wrote:
> Hello
> 
> On 26/11/2021 11:30, Hans de Goede wrote:
>> Hi,
>>
>> On 11/26/21 00:39, Laurent Pinchart wrote:
>>> Hi Hans,
>>>
>>> Thank you for the patch.
>>>
>>> On Thu, Nov 25, 2021 at 05:54:04PM +0100, Hans de Goede wrote:
>>>> From: Daniel Scally <djrscally@gmail.com>
>>>>
>>>> The TPS68470 PMIC has an I2C passthrough mode through which I2C traffic
>>>> can be forwarded to a device connected to the PMIC as though it were
>>>> connected directly to the system bus. Enable this mode when the chip
>>>> is initialised.
>>> Is there any drawback doing this unconditionally, if nothing is
>>> connected to the bus on the other side (including no pull-ups) ?
>> I actually never took a really close look at this patch, I just
>> sorta inherited it from Daniel.
>>
>> Now that I have taken a close look, I see that it is setting the
>> exact same bits as which get set when enabling the VSIO regulator.
>>
>> The idea here is that the I2C-passthrough only gets enabled when
>> the VSIO regulator is turned on, because some sensors end up
>> shorting the I2C pins to ground when the sensor is not powered.
>>
>> Since we set these bits when powering up the VSIO regulator
>> and since we do that before trying to talk to the sensor
>> I don't think that we need this (hack) anymore.
>>
>> I will give things a try without this change and if things
>> still work I will drop this patch from the set.
>>
>> Daniel, what do you think?
> 
> 
> Humm, we're only using the VSIO regulator with the VCM though right?

Nope, there is a mapping from VSIO to dovdd for the ov8865 in the
board_data; and I'm pretty sure I copied that from your earlier
attempts at getting regulator lookups registered :)

And even if the VSIO regulator was only used by the VCM, then it would
get turned off after probing the VCM, clearing the 2 bits which this
commit sets. Which would break things if we did not re-enable it when
the ov8865 needs it.

> Which might not be on when the ov8865 tries to probe. I haven't tried
> without this patch to be honest; I set it because that was what Windows
> does when powering on the PMIC.

See above, I'm pretty sure we can do without this patch which means
that the INT3472 code will no longer be poking at the PMIC directly
itself, which is good :)

Anyways I'll give this a try sometime next week and then drop the
patch.

Regards,

Hans




>>>> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
>>>> Signed-off-by: Daniel Scally <djrscally@gmail.com>
>>>> ---
>>>>  .../x86/intel/int3472/intel_skl_int3472_tps68470.c         | 7 +++++++
>>>>  1 file changed, 7 insertions(+)
>>>>
>>>> diff --git a/drivers/platform/x86/intel/int3472/intel_skl_int3472_tps68470.c b/drivers/platform/x86/intel/int3472/intel_skl_int3472_tps68470.c
>>>> index c05b4cf502fe..42e688f4cad4 100644
>>>> --- a/drivers/platform/x86/intel/int3472/intel_skl_int3472_tps68470.c
>>>> +++ b/drivers/platform/x86/intel/int3472/intel_skl_int3472_tps68470.c
>>>> @@ -45,6 +45,13 @@ static int tps68470_chip_init(struct device *dev, struct regmap *regmap)
>>>>  		return ret;
>>>>  	}
>>>>  
>>>> +	/* Enable I2C daisy chain */
>>>> +	ret = regmap_write(regmap, TPS68470_REG_S_I2C_CTL, 0x03);
>>>> +	if (ret) {
>>>> +		dev_err(dev, "Failed to enable i2c daisy chain\n");
>>>> +		return ret;
>>>> +	}
>>>> +
>>>>  	dev_info(dev, "TPS68470 REVID: 0x%02x\n", version);
>>>>  
>>>>  	return 0;
> 


  reply	other threads:[~2021-11-26 11:48 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-25 16:53 [PATCH v6 00/15] Add support for X86/ACPI camera sensor/PMIC setup with clk and regulator platform data Hans de Goede
2021-11-25 16:53 ` [PATCH v6 01/15] ACPI: delay enumeration of devices with a _DEP pointing to an INT3472 device Hans de Goede
2021-11-25 16:53 ` [PATCH v6 02/15] i2c: acpi: Use acpi_dev_ready_for_enumeration() helper Hans de Goede
2021-11-26  7:16   ` Mika Westerberg
2021-11-25 16:54 ` [PATCH v6 03/15] i2c: acpi: Add i2c_acpi_new_device_by_fwnode() function Hans de Goede
2021-11-25 17:22   ` Andy Shevchenko
2021-11-26 11:19     ` Hans de Goede
2021-11-26  7:18   ` Mika Westerberg
2021-11-27 22:49   ` Wolfram Sang
2021-11-25 16:54 ` [PATCH v6 04/15] platform_data: Add linux/platform_data/tps68470.h file Hans de Goede
2021-11-25 16:54 ` [PATCH v6 05/15] regulator: Introduce tps68470-regulator driver Hans de Goede
2021-11-25 23:32   ` Laurent Pinchart
2021-11-26 11:22     ` Hans de Goede
2021-11-27 23:38       ` Laurent Pinchart
2021-11-29 12:08         ` Mark Brown
2021-11-29 15:35           ` Laurent Pinchart
2021-11-25 16:54 ` [PATCH v6 06/15] clk: Introduce clk-tps68470 driver Hans de Goede
2021-11-25 16:54 ` [PATCH v6 07/15] platform/x86: int3472: Enable I2c daisy chain Hans de Goede
2021-11-25 23:39   ` Laurent Pinchart
2021-11-26 11:30     ` Hans de Goede
2021-11-26 11:39       ` Daniel Scally
2021-11-26 11:45         ` Hans de Goede [this message]
2021-11-26 11:56           ` Daniel Scally
2021-12-03 10:21           ` Hans de Goede
2021-11-25 16:54 ` [PATCH v6 08/15] platform/x86: int3472: Split into 2 drivers Hans de Goede
2021-11-25 16:54 ` [PATCH v6 09/15] platform/x86: int3472: Add get_sensor_adev_and_name() helper Hans de Goede
2021-11-25 16:54 ` [PATCH v6 10/15] platform/x86: int3472: Pass tps68470_clk_platform_data to the tps68470-regulator MFD-cell Hans de Goede
2021-11-25 16:54 ` [PATCH v6 11/15] platform/x86: int3472: Pass tps68470_regulator_platform_data " Hans de Goede
2021-11-25 16:54 ` [PATCH v6 12/15] platform/x86: int3472: Deal with probe ordering issues Hans de Goede
2021-11-25 16:54 ` [PATCH v6 13/15] media: ipu3-cio2: Defer probing until the PMIC is fully setup Hans de Goede
2021-11-25 16:54 ` [PATCH v6 14/15] media: ipu3-cio2: Call cio2_bridge_init() before anything else Hans de Goede
2021-11-25 16:54 ` [PATCH v6 15/15] media: ipu3-cio2: Add support for instantiating i2c-clients for VCMs Hans de Goede

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2fd5400e-e587-54d2-1071-ad8df49a8a68@redhat.com \
    --to=hdegoede@redhat.com \
    --cc=andy.shevchenko@gmail.com \
    --cc=andy@infradead.org \
    --cc=broonie@kernel.org \
    --cc=djrscally@gmail.com \
    --cc=hpa@redhat.com \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=lenb@kernel.org \
    --cc=lgirdwood@gmail.com \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-clk@vger.kernel.org \
    --cc=linux-i2c@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=markgross@kernel.org \
    --cc=mchehab@kernel.org \
    --cc=mika.westerberg@linux.intel.com \
    --cc=mturquette@baylibre.com \
    --cc=platform-driver-x86@vger.kernel.org \
    --cc=rjw@rjwysocki.net \
    --cc=sakari.ailus@linux.intel.com \
    --cc=sboyd@kernel.org \
    --cc=wsa@the-dreams.de \
    --subject='Re: [PATCH v6 07/15] platform/x86: int3472: Enable I2c daisy chain' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).