| Message ID | 1502371530-32260-2-git-send-email-m.purski@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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@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. > [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);
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@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
Hi Marek, On Friday 11 Aug 2017 08:39:31 Marek Szyprowski wrote: > On 2017-08-10 15:39, Laurent Pinchart wrote: > > 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@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? Sure, here you are. &hsi2c_7 { ... sii8620@39 { reg = <0x39>; compatible = "sil,sii8620"; cvcc10-supply = <&ldo36_reg>; iovcc18-supply = <&ldo34_reg>; interrupt-parent = <&gpf0>; interrupts = <2 IRQ_TYPE_LEVEL_HIGH>; reset-gpios = <&gpv7 0 GPIO_ACTIVE_LOW>; clocks = <&pmu_system_controller 0>; clock-names = "xtal"; ports { #address-cells = <1>; #size-cells = <0>; port@0 { reg = <0>; mhl_to_hdmi: endpoint { remote-endpoint = <&hdmi_to_mhl>; }; }; port@1 { reg = <1>; mhl_bridge_out: endpoint { remote-endpoint = <&mhl_connector_in>; }; }; }; }; }; / { mhl-connector { compatible = "mhl-connector"; extcon = <&muic>; port { mhl_connector_in: endpoint { remote-endpoint = <&mhl_bridge_out>; }; }; }; }; Note that there's currently no DT binding for MHL connectors. One needs to be developed, the HDMI connector DT binding can be used as a source of inspiration. > >> [1]: Documentation/devicetree/bindings/media/video-interfaces.txt
Hi Laurent, On 2017-08-11 11:40, Laurent Pinchart wrote: > Hi Marek, > > On Friday 11 Aug 2017 08:39:31 Marek Szyprowski wrote: >> On 2017-08-10 15:39, Laurent Pinchart wrote: >>> 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@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? > Sure, here you are. > > &hsi2c_7 { > ... > > sii8620@39 { > reg = <0x39>; > compatible = "sil,sii8620"; > cvcc10-supply = <&ldo36_reg>; > iovcc18-supply = <&ldo34_reg>; > interrupt-parent = <&gpf0>; > interrupts = <2 IRQ_TYPE_LEVEL_HIGH>; > reset-gpios = <&gpv7 0 GPIO_ACTIVE_LOW>; > clocks = <&pmu_system_controller 0>; > clock-names = "xtal"; > > ports { > #address-cells = <1>; > #size-cells = <0>; > > port@0 { > reg = <0>; > mhl_to_hdmi: endpoint { > remote-endpoint = <&hdmi_to_mhl>; > }; > }; > port@1 { > reg = <1>; > mhl_bridge_out: endpoint { > remote-endpoint = <&mhl_connector_in>; > }; > }; > }; > }; > }; > > / { > mhl-connector { > compatible = "mhl-connector"; > extcon = <&muic>; > > port { > mhl_connector_in: endpoint { > remote-endpoint = <&mhl_bridge_out>; > }; > }; > }; > }; > > Note that there's currently no DT binding for MHL connectors. One needs to be > developed, the HDMI connector DT binding can be used as a source of > inspiration. Ah, I see. I wasn't aware that display subsystem has bindings even for the connectors. Other types of connectors already used together with extcon in device tree (like usb and chargers) don't have dedicated bindings. Do we need to add hdmi-connectors to all existing boards that have HDMI module? >>>> [1]: Documentation/devicetree/bindings/media/video-interfaces.txt >>>> Best regards
On Fri, Aug 11, 2017 at 02:49:35PM +0200, Marek Szyprowski wrote: > Hi Laurent, > > On 2017-08-11 11:40, Laurent Pinchart wrote: > > Hi Marek, > > > > On Friday 11 Aug 2017 08:39:31 Marek Szyprowski wrote: > > > On 2017-08-10 15:39, Laurent Pinchart wrote: > > > > 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@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? > > Sure, here you are. > > > > &hsi2c_7 { > > ... > > > > sii8620@39 { > > reg = <0x39>; > > compatible = "sil,sii8620"; > > cvcc10-supply = <&ldo36_reg>; > > iovcc18-supply = <&ldo34_reg>; > > interrupt-parent = <&gpf0>; > > interrupts = <2 IRQ_TYPE_LEVEL_HIGH>; > > reset-gpios = <&gpv7 0 GPIO_ACTIVE_LOW>; > > clocks = <&pmu_system_controller 0>; > > clock-names = "xtal"; > > > > ports { > > #address-cells = <1>; > > #size-cells = <0>; > > > > port@0 { > > reg = <0>; > > mhl_to_hdmi: endpoint { > > remote-endpoint = <&hdmi_to_mhl>; > > }; > > }; > > port@1 { > > reg = <1>; > > mhl_bridge_out: endpoint { > > remote-endpoint = <&mhl_connector_in>; > > }; > > }; > > }; > > }; > > }; > > > > / { > > mhl-connector { > > compatible = "mhl-connector"; > > extcon = <&muic>; > > > > port { > > mhl_connector_in: endpoint { > > remote-endpoint = <&mhl_bridge_out>; > > }; > > }; > > }; > > }; > > > > Note that there's currently no DT binding for MHL connectors. One needs to be > > developed, the HDMI connector DT binding can be used as a source of > > inspiration. > > Ah, I see. I wasn't aware that display subsystem has bindings even for the > connectors. > Other types of connectors already used together with extcon in device tree > (like usb > and chargers) don't have dedicated bindings. I NAK any extcon binding additions because it is poorly designed, so don't use extcon. It is full of Linuxisms and needs work. USB connectors need to be described like what we have for display related connectors especially since they are merging. > > Do we need to add hdmi-connectors to all existing boards that have HDMI > module? Ideally, yes. Rob
Hi Marek, On Thursday 17 Aug 2017 10:11:02 Rob Herring wrote: > On Fri, Aug 11, 2017 at 02:49:35PM +0200, Marek Szyprowski wrote: > > On 2017-08-11 11:40, Laurent Pinchart wrote: > >> On Friday 11 Aug 2017 08:39:31 Marek Szyprowski wrote: > >>> On 2017-08-10 15:39, Laurent Pinchart wrote: > >>>> 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@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? > >> > >> Sure, here you are. > >> > >> &hsi2c_7 { > >> ... > >> sii8620@39 { > >> reg = <0x39>; > >> compatible = "sil,sii8620"; > >> cvcc10-supply = <&ldo36_reg>; > >> iovcc18-supply = <&ldo34_reg>; > >> interrupt-parent = <&gpf0>; > >> interrupts = <2 IRQ_TYPE_LEVEL_HIGH>; > >> reset-gpios = <&gpv7 0 GPIO_ACTIVE_LOW>; > >> clocks = <&pmu_system_controller 0>; > >> clock-names = "xtal"; > >> > >> ports { > >> #address-cells = <1>; > >> #size-cells = <0>; > >> > >> port@0 { > >> reg = <0>; > >> mhl_to_hdmi: endpoint { > >> remote-endpoint = <&hdmi_to_mhl>; > >> }; > >> }; > >> port@1 { > >> reg = <1>; > >> mhl_bridge_out: endpoint { > >> remote-endpoint = <&mhl_connector_in>; > >> }; > >> }; > >> }; > >> }; > >> }; > >> > >> / { > >> mhl-connector { > >> compatible = "mhl-connector"; > >> extcon = <&muic>; > >> > >> port { > >> mhl_connector_in: endpoint { > >> remote-endpoint = <&mhl_bridge_out>; > >> }; > >> }; > >> }; > >> }; > >> > >> Note that there's currently no DT binding for MHL connectors. One needs > >> to be developed, the HDMI connector DT binding can be used as a source > >> of inspiration. > > > > Ah, I see. I wasn't aware that display subsystem has bindings even for the > > connectors. Other types of connectors already used together with extcon in > > device tree (like usb and chargers) don't have dedicated bindings. > > I NAK any extcon binding additions because it is poorly designed, so > don't use extcon. It is full of Linuxisms and needs work. USB > connectors need to be described like what we have for display related > connectors especially since they are merging. > > > Do we need to add hdmi-connectors to all existing boards that have HDMI > > module? > > Ideally, yes. I agree. In practice "details" such as backward compatibility need to be taken into account, but it's a good practice to update the mainline device tree sources to the recommended bindings.
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 [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);
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@samsung.com> --- .../bindings/display/bridge/sil-sii8620.txt | 4 ++ drivers/gpu/drm/bridge/sil-sii8620.c | 83 +++++++++++++++++++++- 2 files changed, 85 insertions(+), 2 deletions(-)