2020-02-24 09:16:21

by Maxime Ripard

[permalink] [raw]
Subject: [PATCH 03/89] i2c: brcmstb: Support BCM2711 HDMI BSC controllers

The HDMI blocks in the BCM2771 have an i2c controller to retrieve the
EDID. This block is split into two parts, the BSC and the AUTO_I2C,
lying in two separate register areas.

The AUTO_I2C block has a mailbox-like interface and will take away the
BSC control from the CPU if enabled. However, the BSC is the actually
the same controller than the one supported by the brcmstb driver, and
the AUTO_I2C doesn't really bring any immediate benefit.

Let's use the BSC then, but let's also tie the AUTO_I2C registers with a
separate compatible so that we can enable AUTO_I2C if needed in the
future.

The AUTO_I2C is enabled by default at boot though, so we first need to
release the BSC from the AUTO_I2C control.

Cc: Kamal Dasu <[email protected]>
Cc: Florian Fainelli <[email protected]>
Cc: Wolfram Sang <[email protected]>
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Maxime Ripard <[email protected]>
---
drivers/i2c/busses/i2c-brcmstb.c | 33 +++++++++++++++++++++++++++++++++-
1 file changed, 33 insertions(+)

diff --git a/drivers/i2c/busses/i2c-brcmstb.c b/drivers/i2c/busses/i2c-brcmstb.c
index 506991596b68..169a2836922d 100644
--- a/drivers/i2c/busses/i2c-brcmstb.c
+++ b/drivers/i2c/busses/i2c-brcmstb.c
@@ -580,6 +580,31 @@ static void brcmstb_i2c_set_bsc_reg_defaults(struct brcmstb_i2c_dev *dev)
brcmstb_i2c_set_bus_speed(dev);
}

+#define AUTOI2C_CTRL0 0x26c
+#define AUTOI2C_CTRL0_RELEASE_BSC BIT(1)
+
+static int bcm2711_release_bsc(struct brcmstb_i2c_dev *dev)
+{
+ struct platform_device *pdev = to_platform_device(dev->device);
+ struct resource *iomem;
+ void __iomem *autoi2c;
+
+ /* Map hardware registers */
+ iomem = platform_get_resource_byname(pdev, IORESOURCE_MEM, "auto-i2c");
+ autoi2c = devm_ioremap_resource(&pdev->dev, iomem);
+ if (IS_ERR(autoi2c))
+ return PTR_ERR(autoi2c);
+
+ writel(AUTOI2C_CTRL0_RELEASE_BSC, autoi2c + AUTOI2C_CTRL0);
+ devm_iounmap(&pdev->dev, autoi2c);
+
+ /* We need to reset the controller after the release */
+ dev->bsc_regmap->iic_enable = 0;
+ bsc_writel(dev, dev->bsc_regmap->iic_enable, iic_enable);
+
+ return 0;
+}
+
static int brcmstb_i2c_probe(struct platform_device *pdev)
{
int rc = 0;
@@ -609,6 +634,13 @@ static int brcmstb_i2c_probe(struct platform_device *pdev)
goto probe_errorout;
}

+ if (of_device_is_compatible(dev->device->of_node,
+ "brcm,bcm2711-hdmi-i2c")) {
+ rc = bcm2711_release_bsc(dev);
+ if (rc)
+ goto probe_errorout;
+ }
+
rc = of_property_read_string(dev->device->of_node, "interrupt-names",
&int_name);
if (rc < 0)
@@ -705,6 +737,7 @@ static SIMPLE_DEV_PM_OPS(brcmstb_i2c_pm, brcmstb_i2c_suspend,
static const struct of_device_id brcmstb_i2c_of_match[] = {
{.compatible = "brcm,brcmstb-i2c"},
{.compatible = "brcm,brcmper-i2c"},
+ {.compatible = "brcm,bcm2711-hdmi-i2c"},
{},
};
MODULE_DEVICE_TABLE(of, brcmstb_i2c_of_match);
--
git-series 0.9.1


2020-02-24 17:46:31

by Florian Fainelli

[permalink] [raw]
Subject: Re: [PATCH 03/89] i2c: brcmstb: Support BCM2711 HDMI BSC controllers

On 2/24/20 1:06 AM, Maxime Ripard wrote:
> The HDMI blocks in the BCM2771 have an i2c controller to retrieve the
> EDID. This block is split into two parts, the BSC and the AUTO_I2C,
> lying in two separate register areas.
>
> The AUTO_I2C block has a mailbox-like interface and will take away the
> BSC control from the CPU if enabled. However, the BSC is the actually
> the same controller than the one supported by the brcmstb driver, and
> the AUTO_I2C doesn't really bring any immediate benefit.
>
> Let's use the BSC then, but let's also tie the AUTO_I2C registers with a
> separate compatible so that we can enable AUTO_I2C if needed in the
> future.
>
> The AUTO_I2C is enabled by default at boot though, so we first need to
> release the BSC from the AUTO_I2C control.
>
> Cc: Kamal Dasu <[email protected]>
> Cc: Florian Fainelli <[email protected]>
> Cc: Wolfram Sang <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: Maxime Ripard <[email protected]>

[snip]

> @@ -705,6 +737,7 @@ static SIMPLE_DEV_PM_OPS(brcmstb_i2c_pm, brcmstb_i2c_suspend,
> static const struct of_device_id brcmstb_i2c_of_match[] = {
> {.compatible = "brcm,brcmstb-i2c"},
> {.compatible = "brcm,brcmper-i2c"},
> + {.compatible = "brcm,bcm2711-hdmi-i2c"},

You could have added the bcm2711_release_bsc here as a function attached
with the of_device_id::data member of the structure and do:

if (data && data->init_func)
rc = data->init_func(dev);

But we can defer that until we have a second compatible string that
requires the same approach.

Akked-by: Florian Fainelli <[email protected]>
--
Florian

2020-03-10 10:13:14

by Wolfram Sang

[permalink] [raw]
Subject: Re: [PATCH 03/89] i2c: brcmstb: Support BCM2711 HDMI BSC controllers

On Mon, Feb 24, 2020 at 10:06:05AM +0100, Maxime Ripard wrote:
> The HDMI blocks in the BCM2771 have an i2c controller to retrieve the
> EDID. This block is split into two parts, the BSC and the AUTO_I2C,
> lying in two separate register areas.
>
> The AUTO_I2C block has a mailbox-like interface and will take away the
> BSC control from the CPU if enabled. However, the BSC is the actually
> the same controller than the one supported by the brcmstb driver, and
> the AUTO_I2C doesn't really bring any immediate benefit.
>
> Let's use the BSC then, but let's also tie the AUTO_I2C registers with a
> separate compatible so that we can enable AUTO_I2C if needed in the
> future.
>
> The AUTO_I2C is enabled by default at boot though, so we first need to
> release the BSC from the AUTO_I2C control.
>
> Cc: Kamal Dasu <[email protected]>
> Cc: Florian Fainelli <[email protected]>
> Cc: Wolfram Sang <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: Maxime Ripard <[email protected]>

Fixed the acked-by and applied to for-next, thanks!

FYI, cppcheck rightfully warned about this in the driver:

drivers/i2c/busses/i2c-brcmstb.c:319:7: warning: Condition 'CMD_RD' is always true [knownConditionTrueFalse]
if ((CMD_RD || CMD_WR) &&
^
drivers/i2c/busses/i2c-brcmstb.c:319:17: warning: Condition 'CMD_WR' is always false [knownConditionTrueFalse]
if ((CMD_RD || CMD_WR) &&
^
drivers/i2c/busses/i2c-brcmstb.c:464:0: warning: Variable 'len' is assigned a value that is never used. [unreadVariable]
int len = 0;

Not related to this patch, but maybe one of you is interested...


Attachments:
(No filename) (1.67 kB)
signature.asc (849.00 B)
Download all attachments