[PATCH 1/2] drm/bridge/sii8620: add external connector handle

[Marek Szyprowski]

Hi Laurent,

On 2017-08-10 15:39, Laurent Pinchart wrote:
> Hi Maciej,
>
> Thank you for the patch.
>
> On Thursday 10 Aug 2017 15:25:29 Maciej Purski wrote:
>> The driver should be switched on if an external connector is plugged and
>> switched off if it is unplugged. Extcon is optional. If it is not found,
>> the driver stays in "always-on" mode.
>>
>> Signed-off-by: Maciej Purski <m.purski at samsung.com>
>> ---
>>   .../bindings/display/bridge/sil-sii8620.txt        |  4 ++
>>   drivers/gpu/drm/bridge/sil-sii8620.c               | 83 ++++++++++++++++++-
>>   2 files changed, 85 insertions(+), 2 deletions(-)
>>
>> diff --git
>> a/Documentation/devicetree/bindings/display/bridge/sil-sii8620.txt
>> b/Documentation/devicetree/bindings/display/bridge/sil-sii8620.txt index
>> 9409d9c..1f230bf 100644
>> --- a/Documentation/devicetree/bindings/display/bridge/sil-sii8620.txt
>> +++ b/Documentation/devicetree/bindings/display/bridge/sil-sii8620.txt
>> @@ -10,6 +10,9 @@ Required properties:
>>   	- clocks, clock-names: specification and name of "xtal" clock
>>   	- video interfaces: Device node can contain video interface port
>>   			    node for HDMI encoder according to [1].
>> +Optional properties:
>> +	- extcon: phandle to external connector for MHL cable changes
>> +		  detection
> The sii8620 DT node should model its connection to the MHL connector using OF
> graph, connecting a port to the MHL connector DT node through endpoints. I
> believe the extcon property should then be added to the MHL connector DT node,
> not the bridge.

I don't get this. Generic extcon bindings doesn't define connector DT 
node, so
I don't see how could we use endpoint DT approach here. Could you give the
example?

>>   [1]: Documentation/devicetree/bindings/media/video-interfaces.txt
>>
>> @@ -24,6 +27,7 @@ Example:
>>   		reset-gpio = <&gpv7 0 0>;
>>   		clocks = <&pmu_system_controller 0>;
>>   		clock-names = "xtal";
>> +		extcon = <&muic>;
>>
>>   		port {
>>   			mhl_to_hdmi: endpoint {
>> diff --git a/drivers/gpu/drm/bridge/sil-sii8620.c
>> b/drivers/gpu/drm/bridge/sil-sii8620.c index 2d51a22..5002654 100644
>> --- a/drivers/gpu/drm/bridge/sil-sii8620.c
>> +++ b/drivers/gpu/drm/bridge/sil-sii8620.c
>> @@ -17,6 +17,7 @@
>>
>>   #include <linux/clk.h>
>>   #include <linux/delay.h>
>> +#include <linux/extcon.h>
>>   #include <linux/gpio/consumer.h>
>>   #include <linux/i2c.h>
>>   #include <linux/interrupt.h>
>> @@ -78,6 +79,10 @@ struct sii8620 {
>>   	struct edid *edid;
>>   	unsigned int gen2_write_burst:1;
>>   	enum sii8620_mt_state mt_state;
>> +	struct extcon_dev *extcon;
>> +	struct notifier_block extcon_nb;
>> +	struct work_struct extcon_wq;
>> +	bool extcon_attached;
>>   	struct list_head mt_queue;
>>   	struct {
>>   		int r_size;
>> @@ -2102,6 +2107,72 @@ static void sii8620_cable_in(struct sii8620 *ctx)
>>   	enable_irq(to_i2c_client(ctx->dev)->irq);
>>   }
>>
>> +static void sii8620_cable_out(struct sii8620 *ctx)
>> +{
>> +	disable_irq(to_i2c_client(ctx->dev)->irq);
>> +	sii8620_hw_off(ctx);
>> +}
>> +
>> +static void sii8620_extcon_work(struct work_struct *work)
>> +{
>> +	struct sii8620 *ctx =
>> +		container_of(work, struct sii8620, extcon_wq);
>> +
>> +	if (ctx->extcon_attached)
>> +		sii8620_cable_in(ctx);
>> +	else
>> +		sii8620_cable_out(ctx);
>> +}
>> +
>> +static int sii8620_extcon_notifier(struct notifier_block *self,
>> +			unsigned long event, void *ptr)
>> +{
>> +	struct sii8620 *ctx =
>> +		container_of(self, struct sii8620, extcon_nb);
>> +
>> +	ctx->extcon_attached = event;
>> +	schedule_work(&ctx->extcon_wq);
>> +
>> +	return NOTIFY_DONE;
>> +}
>> +
>> +static int sii8620_extcon_init(struct sii8620 *ctx)
>> +{
>> +	struct extcon_dev *edev;
>> +	int ret;
>> +
>> +	INIT_WORK(&ctx->extcon_wq, sii8620_extcon_work);
>> +
>> +	if (!of_property_read_bool(ctx->dev->of_node, "extcon")) {
>> +		dev_info(ctx->dev, "no extcon found, switching to 'always on'
> mode\n");
>> +		ctx->extcon_attached = true;
>> +		return 0;
>> +	}
>> +
>> +	edev = extcon_get_edev_by_phandle(ctx->dev, 0);
>> +	if (IS_ERR(edev)) {
>> +		if (PTR_ERR(edev) == -EPROBE_DEFER)
>> +			return -EPROBE_DEFER;
>> +		dev_err(ctx->dev, "Invalid or missing extcon\n");
>> +		return PTR_ERR(edev);
>> +	}
>> +
>> +	ctx->extcon_attached = extcon_get_cable_state_(edev, EXTCON_DISP_MHL);
>> +	dev_info(ctx->dev, "extcon(MHL) = %d\n", ctx->extcon_attached);
>> +
>> +	ctx->extcon = edev;
>> +	ctx->extcon_nb.notifier_call = sii8620_extcon_notifier;
>> +	ret = devm_extcon_register_notifier(ctx->dev, edev,
>> +					EXTCON_DISP_MHL, &ctx->extcon_nb);
>> +
>> +	if (ret) {
>> +		dev_err(ctx->dev, "failed to register notifier for MHL\n");
>> +		return ret;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>>   static inline struct sii8620 *bridge_to_sii8620(struct drm_bridge *bridge)
>>   {
>>   	return container_of(bridge, struct sii8620, bridge);
>> @@ -2201,13 +2272,20 @@ static int sii8620_probe(struct i2c_client *client,
>>   	if (ret)
>>   		return ret;
>>
>> +	ret = sii8620_extcon_init(ctx);
>> +	if (ret < 0) {
>> +		dev_err(ctx->dev, "failed to initialize EXTCON\n");
>> +		return ret;
>> +	}
>> +
>>   	i2c_set_clientdata(client, ctx);
>>
>>   	ctx->bridge.funcs = &sii8620_bridge_funcs;
>>   	ctx->bridge.of_node = dev->of_node;
>>   	drm_bridge_add(&ctx->bridge);
>>
>> -	sii8620_cable_in(ctx);
>> +	if (ctx->extcon_attached)
>> +		sii8620_cable_in(ctx);
>>
>>   	return 0;
>>   }
>> @@ -2216,7 +2294,8 @@ static int sii8620_remove(struct i2c_client *client)
>>   {
>>   	struct sii8620 *ctx = i2c_get_clientdata(client);
>>
>> -	disable_irq(to_i2c_client(ctx->dev)->irq);
>> +	if (!ctx->extcon_attached)
>> +		sii8620_cable_out(ctx);
>>   	drm_bridge_remove(&ctx->bridge);
>>   	sii8620_hw_off(ctx);

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland