Re: [PATCHv3 0/2] pinctrl: sh-pfc: r8a7796: Add drive strength and bias support

2016-11-29 Thread Geert Uytterhoeven
On Thu, Nov 17, 2016 at 4:09 PM, Niklas Söderlund
 wrote:
> These patches add drive strength and bias support for both GPIO and none
> GPIO pins to r8a7796. Similar to the none GPIO pins for r8a7795 the
> system to derive unique pin numbers are the R-Car M3SiP pin layout.
>
> Tested on M3-W and the series depends on the series '[PATCHv3 0/6]
> pinctrl: sh-pfc: Fixups for bias handling and preparation for none GPIO
> pins'.

Thanks, queued in sh-pfc-for-v4.11.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds


[PATCH 06/12] usb: dwc3: omap: Replace the extcon API

2016-11-29 Thread Chanwoo Choi
This patch uses the resource-managed extcon API for extcon_register_notifier()
and replaces the deprecated extcon API as following:
- extcon_get_cable_state_() -> extcon_get_state()

Signed-off-by: Chanwoo Choi 
---
 drivers/usb/dwc3/dwc3-omap.c | 20 +++-
 1 file changed, 7 insertions(+), 13 deletions(-)

diff --git a/drivers/usb/dwc3/dwc3-omap.c b/drivers/usb/dwc3/dwc3-omap.c
index 29e80cc9b634..2d2e9aa1db08 100644
--- a/drivers/usb/dwc3/dwc3-omap.c
+++ b/drivers/usb/dwc3/dwc3-omap.c
@@ -425,20 +425,20 @@ static int dwc3_omap_extcon_register(struct dwc3_omap 
*omap)
}
 
omap->vbus_nb.notifier_call = dwc3_omap_vbus_notifier;
-   ret = extcon_register_notifier(edev, EXTCON_USB,
-   &omap->vbus_nb);
+   ret = devm_extcon_register_notifier(omap->dev, edev,
+   EXTCON_USB, &omap->vbus_nb);
if (ret < 0)
dev_vdbg(omap->dev, "failed to register notifier for 
USB\n");
 
omap->id_nb.notifier_call = dwc3_omap_id_notifier;
-   ret = extcon_register_notifier(edev, EXTCON_USB_HOST,
-   &omap->id_nb);
+   ret = devm_extcon_register_notifier(omap->dev, edev,
+   EXTCON_USB_HOST, &omap->id_nb);
if (ret < 0)
dev_vdbg(omap->dev, "failed to register notifier for 
USB-HOST\n");
 
-   if (extcon_get_cable_state_(edev, EXTCON_USB) == true)
+   if (extcon_get_state(edev, EXTCON_USB) == true)
dwc3_omap_set_mailbox(omap, OMAP_DWC3_VBUS_VALID);
-   if (extcon_get_cable_state_(edev, EXTCON_USB_HOST) == true)
+   if (extcon_get_state(edev, EXTCON_USB_HOST) == true)
dwc3_omap_set_mailbox(omap, OMAP_DWC3_ID_GROUND);
 
omap->edev = edev;
@@ -527,17 +527,13 @@ static int dwc3_omap_probe(struct platform_device *pdev)
ret = of_platform_populate(node, NULL, NULL, dev);
if (ret) {
dev_err(&pdev->dev, "failed to create dwc3 core\n");
-   goto err2;
+   goto err1;
}
 
dwc3_omap_enable_irqs(omap);
 
return 0;
 
-err2:
-   extcon_unregister_notifier(omap->edev, EXTCON_USB, &omap->vbus_nb);
-   extcon_unregister_notifier(omap->edev, EXTCON_USB_HOST, &omap->id_nb);
-
 err1:
pm_runtime_put_sync(dev);
pm_runtime_disable(dev);
@@ -549,8 +545,6 @@ static int dwc3_omap_remove(struct platform_device *pdev)
 {
struct dwc3_omap*omap = platform_get_drvdata(pdev);
 
-   extcon_unregister_notifier(omap->edev, EXTCON_USB, &omap->vbus_nb);
-   extcon_unregister_notifier(omap->edev, EXTCON_USB_HOST, &omap->id_nb);
dwc3_omap_disable_irqs(omap);
of_platform_depopulate(omap->dev);
pm_runtime_put_sync(&pdev->dev);
-- 
1.9.1



[PATCH 02/12] phy: sun4i-usb: Replace the deprecated extcon API

2016-11-29 Thread Chanwoo Choi
This patch replaces the deprecated extcon API as following:
- extcon_set_cable_state_() -> extcon_set_state_sync()

Cc: Kishon Vijay Abraham I 
Cc: Maxime Ripard 
Cc: Chen-Yu Tsai 
Signed-off-by: Chanwoo Choi 
---
 drivers/phy/phy-sun4i-usb.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/phy/phy-sun4i-usb.c b/drivers/phy/phy-sun4i-usb.c
index eeda5134c777..95cdb08c339f 100644
--- a/drivers/phy/phy-sun4i-usb.c
+++ b/drivers/phy/phy-sun4i-usb.c
@@ -528,7 +528,7 @@ static void sun4i_usb_phy0_id_vbus_det_scan(struct 
work_struct *work)
mutex_unlock(&phy0->mutex);
 
if (id_notify) {
-   extcon_set_cable_state_(data->extcon, EXTCON_USB_HOST,
+   extcon_set_state_sync(data->extcon, EXTCON_USB_HOST,
!id_det);
/* When leaving host mode force end the session here */
if (force_session_end && id_det == 1) {
@@ -541,7 +541,7 @@ static void sun4i_usb_phy0_id_vbus_det_scan(struct 
work_struct *work)
}
 
if (vbus_notify)
-   extcon_set_cable_state_(data->extcon, EXTCON_USB, vbus_det);
+   extcon_set_state_sync(data->extcon, EXTCON_USB, vbus_det);
 
if (sun4i_usb_phy0_poll(data))
queue_delayed_work(system_wq, &data->detect, POLL_TIME);
-- 
1.9.1



[PATCH 08/12] usb: phy: msm: Replace the extcon API

2016-11-29 Thread Chanwoo Choi
This patch uses the resource-managed extcon API for extcon_register_notifier()
and replaces the deprecated extcon API as following:
- extcon_get_cable_state_() -> extcon_get_state()

Signed-off-by: Chanwoo Choi 
---
 drivers/usb/phy/phy-msm-usb.c | 33 +++--
 1 file changed, 11 insertions(+), 22 deletions(-)

diff --git a/drivers/usb/phy/phy-msm-usb.c b/drivers/usb/phy/phy-msm-usb.c
index 8a34759727bb..a15a89d4235d 100644
--- a/drivers/usb/phy/phy-msm-usb.c
+++ b/drivers/usb/phy/phy-msm-usb.c
@@ -1742,14 +1742,14 @@ static int msm_otg_read_dt(struct platform_device 
*pdev, struct msm_otg *motg)
if (!IS_ERR(ext_vbus)) {
motg->vbus.extcon = ext_vbus;
motg->vbus.nb.notifier_call = msm_otg_vbus_notifier;
-   ret = extcon_register_notifier(ext_vbus, EXTCON_USB,
-   &motg->vbus.nb);
+   ret = devm_extcon_register_notifier(&pdev->dev, ext_vbus,
+   EXTCON_USB, &motg->vbus.nb);
if (ret < 0) {
dev_err(&pdev->dev, "register VBUS notifier failed\n");
return ret;
}
 
-   ret = extcon_get_cable_state_(ext_vbus, EXTCON_USB);
+   ret = extcon_get_state(ext_vbus, EXTCON_USB);
if (ret)
set_bit(B_SESS_VLD, &motg->inputs);
else
@@ -1759,16 +1759,14 @@ static int msm_otg_read_dt(struct platform_device 
*pdev, struct msm_otg *motg)
if (!IS_ERR(ext_id)) {
motg->id.extcon = ext_id;
motg->id.nb.notifier_call = msm_otg_id_notifier;
-   ret = extcon_register_notifier(ext_id, EXTCON_USB_HOST,
-   &motg->id.nb);
+   ret = devm_extcon_register_notifier(&pdev->dev, ext_id,
+   EXTCON_USB_HOST, &motg->id.nb);
if (ret < 0) {
dev_err(&pdev->dev, "register ID notifier failed\n");
-   extcon_unregister_notifier(motg->vbus.extcon,
-  EXTCON_USB, &motg->vbus.nb);
return ret;
}
 
-   ret = extcon_get_cable_state_(ext_id, EXTCON_USB_HOST);
+   ret = extcon_get_state(ext_id, EXTCON_USB_HOST);
if (ret)
clear_bit(ID, &motg->inputs);
else
@@ -1883,10 +1881,9 @@ static int msm_otg_probe(struct platform_device *pdev)
 */
if (motg->phy_number) {
phy_select = devm_ioremap_nocache(&pdev->dev, USB2_PHY_SEL, 4);
-   if (!phy_select) {
-   ret = -ENOMEM;
-   goto unregister_extcon;
-   }
+   if (!phy_select)
+   return -ENOMEM;
+
/* Enable second PHY with the OTG port */
writel(0x1, phy_select);
}
@@ -1897,7 +1894,7 @@ static int msm_otg_probe(struct platform_device *pdev)
if (motg->irq < 0) {
dev_err(&pdev->dev, "platform_get_irq failed\n");
ret = motg->irq;
-   goto unregister_extcon;
+   return motg->irq;
}
 
regs[0].supply = "vddcx";
@@ -1906,7 +1903,7 @@ static int msm_otg_probe(struct platform_device *pdev)
 
ret = devm_regulator_bulk_get(motg->phy.dev, ARRAY_SIZE(regs), regs);
if (ret)
-   goto unregister_extcon;
+   return ret;
 
motg->vddcx = regs[0].consumer;
motg->v3p3  = regs[1].consumer;
@@ -2003,11 +2000,6 @@ static int msm_otg_probe(struct platform_device *pdev)
clk_disable_unprepare(motg->clk);
if (!IS_ERR(motg->core_clk))
clk_disable_unprepare(motg->core_clk);
-unregister_extcon:
-   extcon_unregister_notifier(motg->id.extcon,
-  EXTCON_USB_HOST, &motg->id.nb);
-   extcon_unregister_notifier(motg->vbus.extcon,
-  EXTCON_USB, &motg->vbus.nb);
 
return ret;
 }
@@ -2029,9 +2021,6 @@ static int msm_otg_remove(struct platform_device *pdev)
 */
gpiod_set_value_cansleep(motg->switch_gpio, 0);
 
-   extcon_unregister_notifier(motg->id.extcon, EXTCON_USB_HOST, 
&motg->id.nb);
-   extcon_unregister_notifier(motg->vbus.extcon, EXTCON_USB, 
&motg->vbus.nb);
-
msm_otg_debugfs_cleanup();
cancel_delayed_work_sync(&motg->chg_work);
cancel_work_sync(&motg->sm_work);
-- 
1.9.1



[PATCH 10/12] usb: phy: qcom-8x16-usb: Replace the extcon API

2016-11-29 Thread Chanwoo Choi
This patch uses the resource-managed extcon API for extcon_register_notifier()
and replaces the deprecated extcon API as following:
- extcon_get_cable_state_() -> extcon_get_state()

Signed-off-by: Chanwoo Choi 
---
 drivers/usb/phy/phy-qcom-8x16-usb.c | 13 -
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/drivers/usb/phy/phy-qcom-8x16-usb.c 
b/drivers/usb/phy/phy-qcom-8x16-usb.c
index d8593adb3621..fdf686398772 100644
--- a/drivers/usb/phy/phy-qcom-8x16-usb.c
+++ b/drivers/usb/phy/phy-qcom-8x16-usb.c
@@ -187,7 +187,7 @@ static int phy_8x16_init(struct usb_phy *phy)
val = ULPI_PWR_OTG_COMP_DISABLE;
usb_phy_io_write(phy, val, ULPI_SET(ULPI_PWR_CLK_MNG_REG));
 
-   state = extcon_get_cable_state_(qphy->vbus_edev, EXTCON_USB);
+   state = extcon_get_state(qphy->vbus_edev, EXTCON_USB);
if (state)
phy_8x16_vbus_on(qphy);
else
@@ -316,23 +316,20 @@ static int phy_8x16_probe(struct platform_device *pdev)
goto off_clks;
 
qphy->vbus_notify.notifier_call = phy_8x16_vbus_notify;
-   ret = extcon_register_notifier(qphy->vbus_edev, EXTCON_USB,
-  &qphy->vbus_notify);
+   ret = devm_extcon_register_notifier(&pdev->dev, qphy->vbus_edev,
+   EXTCON_USB, &qphy->vbus_notify);
if (ret < 0)
goto off_power;
 
ret = usb_add_phy_dev(&qphy->phy);
if (ret)
-   goto off_extcon;
+   goto off_power;
 
qphy->reboot_notify.notifier_call = phy_8x16_reboot_notify;
register_reboot_notifier(&qphy->reboot_notify);
 
return 0;
 
-off_extcon:
-   extcon_unregister_notifier(qphy->vbus_edev, EXTCON_USB,
-  &qphy->vbus_notify);
 off_power:
regulator_bulk_disable(ARRAY_SIZE(qphy->regulator), qphy->regulator);
 off_clks:
@@ -347,8 +344,6 @@ static int phy_8x16_remove(struct platform_device *pdev)
struct phy_8x16 *qphy = platform_get_drvdata(pdev);
 
unregister_reboot_notifier(&qphy->reboot_notify);
-   extcon_unregister_notifier(qphy->vbus_edev, EXTCON_USB,
-  &qphy->vbus_notify);
 
/*
 * Ensure that D+/D- lines are routed to uB connector, so
-- 
1.9.1



[PATCH 12/12] usb: renesas_usbhs: Replace the deprecated extcon API

2016-11-29 Thread Chanwoo Choi
This patch replaces the deprecated extcon API as following:
- extcon_get_cable_state_() -> extcon_get_state()

Signed-off-by: Chanwoo Choi 
---
 drivers/usb/renesas_usbhs/common.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/renesas_usbhs/common.c 
b/drivers/usb/renesas_usbhs/common.c
index 012a37aa3e0d..623c51300393 100644
--- a/drivers/usb/renesas_usbhs/common.c
+++ b/drivers/usb/renesas_usbhs/common.c
@@ -389,7 +389,7 @@ static void usbhsc_hotplug(struct usbhs_priv *priv)
 
if (enable && !mod) {
if (priv->edev) {
-   cable = extcon_get_cable_state_(priv->edev, 
EXTCON_USB_HOST);
+   cable = extcon_get_state(priv->edev, EXTCON_USB_HOST);
if ((cable > 0 && id != USBHS_HOST) ||
(!cable && id != USBHS_GADGET)) {
dev_info(&pdev->dev,
-- 
1.9.1



[PATCH 04/12] power_supply: qcom_smbb: Replace the deprecated extcon API

2016-11-29 Thread Chanwoo Choi
This patch replaces the deprecated extcon API as following:
- extcon_set_cable_state_() -> extcon_set_state_sync()

Signed-off-by: Chanwoo Choi 
---
 drivers/power/supply/qcom_smbb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/power/supply/qcom_smbb.c b/drivers/power/supply/qcom_smbb.c
index bb91a1c339bc..7b1991265b03 100644
--- a/drivers/power/supply/qcom_smbb.c
+++ b/drivers/power/supply/qcom_smbb.c
@@ -378,7 +378,7 @@ static irqreturn_t smbb_usb_valid_handler(int irq, void 
*_data)
struct smbb_charger *chg = _data;
 
smbb_set_line_flag(chg, irq, STATUS_USBIN_VALID);
-   extcon_set_cable_state_(chg->edev, EXTCON_USB,
+   extcon_set_state_sync(chg->edev, EXTCON_USB,
chg->status & STATUS_USBIN_VALID);
power_supply_changed(chg->usb_psy);
 
-- 
1.9.1



[PATCH 09/12] usb: phy: omap-otg: Replace the extcon API

2016-11-29 Thread Chanwoo Choi
This patch uses the resource-managed extcon API for extcon_register_notifier()
and replaces the deprecated extcon API as following:
- extcon_get_cable_state_() -> extcon_get_state()

Signed-off-by: Chanwoo Choi 
---
 drivers/usb/phy/phy-omap-otg.c | 24 ++--
 1 file changed, 6 insertions(+), 18 deletions(-)

diff --git a/drivers/usb/phy/phy-omap-otg.c b/drivers/usb/phy/phy-omap-otg.c
index 6523af4f8f93..800d1d90753d 100644
--- a/drivers/usb/phy/phy-omap-otg.c
+++ b/drivers/usb/phy/phy-omap-otg.c
@@ -118,19 +118,19 @@ static int omap_otg_probe(struct platform_device *pdev)
otg_dev->id_nb.notifier_call = omap_otg_id_notifier;
otg_dev->vbus_nb.notifier_call = omap_otg_vbus_notifier;
 
-   ret = extcon_register_notifier(extcon, EXTCON_USB_HOST, 
&otg_dev->id_nb);
+   ret = devm_extcon_register_notifier(&pdev->dev, extcon,
+   EXTCON_USB_HOST, &otg_dev->id_nb);
if (ret)
return ret;
 
-   ret = extcon_register_notifier(extcon, EXTCON_USB, &otg_dev->vbus_nb);
+   ret = devm_extcon_register_notifier(&pdev->dev, extcon,
+   EXTCON_USB, &otg_dev->vbus_nb);
if (ret) {
-   extcon_unregister_notifier(extcon, EXTCON_USB_HOST,
-   &otg_dev->id_nb);
return ret;
}
 
-   otg_dev->id = extcon_get_cable_state_(extcon, EXTCON_USB_HOST);
-   otg_dev->vbus = extcon_get_cable_state_(extcon, EXTCON_USB);
+   otg_dev->id = extcon_get_state(extcon, EXTCON_USB_HOST);
+   otg_dev->vbus = extcon_get_state(extcon, EXTCON_USB);
omap_otg_set_mode(otg_dev);
 
rev = readl(otg_dev->base);
@@ -145,20 +145,8 @@ static int omap_otg_probe(struct platform_device *pdev)
return 0;
 }
 
-static int omap_otg_remove(struct platform_device *pdev)
-{
-   struct otg_device *otg_dev = platform_get_drvdata(pdev);
-   struct extcon_dev *edev = otg_dev->extcon;
-
-   extcon_unregister_notifier(edev, EXTCON_USB_HOST, &otg_dev->id_nb);
-   extcon_unregister_notifier(edev, EXTCON_USB, &otg_dev->vbus_nb);
-
-   return 0;
-}
-
 static struct platform_driver omap_otg_driver = {
.probe  = omap_otg_probe,
-   .remove = omap_otg_remove,
.driver = {
.name   = "omap_otg",
},
-- 
1.9.1



[PATCH 05/12] usb: chipdata: Replace the extcon API

2016-11-29 Thread Chanwoo Choi
This patch uses the resource-managed extcon API for extcon_register_notifier()
and replaces the deprecated extcon API as following:
- extcon_get_cable_state_() -> extcon_get_state()

Signed-off-by: Chanwoo Choi 
---
 drivers/usb/chipidea/core.c | 30 ++
 1 file changed, 6 insertions(+), 24 deletions(-)

diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index 69426e644d17..a5b44963eaea 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -742,7 +742,7 @@ static int ci_get_platdata(struct device *dev,
cable->edev = ext_vbus;
 
if (!IS_ERR(ext_vbus)) {
-   ret = extcon_get_cable_state_(cable->edev, EXTCON_USB);
+   ret = extcon_get_state(cable->edev, EXTCON_USB);
if (ret)
cable->state = true;
else
@@ -754,7 +754,7 @@ static int ci_get_platdata(struct device *dev,
cable->edev = ext_id;
 
if (!IS_ERR(ext_id)) {
-   ret = extcon_get_cable_state_(cable->edev, EXTCON_USB_HOST);
+   ret = extcon_get_state(cable->edev, EXTCON_USB_HOST);
if (ret)
cable->state = false;
else
@@ -771,8 +771,8 @@ static int ci_extcon_register(struct ci_hdrc *ci)
id = &ci->platdata->id_extcon;
id->ci = ci;
if (!IS_ERR(id->edev)) {
-   ret = extcon_register_notifier(id->edev, EXTCON_USB_HOST,
-  &id->nb);
+   ret = devm_extcon_register_notifier(ci->dev, id->edev,
+   EXTCON_USB_HOST, &id->nb);
if (ret < 0) {
dev_err(ci->dev, "register ID failed\n");
return ret;
@@ -782,11 +782,9 @@ static int ci_extcon_register(struct ci_hdrc *ci)
vbus = &ci->platdata->vbus_extcon;
vbus->ci = ci;
if (!IS_ERR(vbus->edev)) {
-   ret = extcon_register_notifier(vbus->edev, EXTCON_USB,
-  &vbus->nb);
+   ret = devm_extcon_register_notifier(ci->dev, vbus->edev,
+   EXTCON_USB, &vbus->nb);
if (ret < 0) {
-   extcon_unregister_notifier(id->edev, EXTCON_USB_HOST,
-  &id->nb);
dev_err(ci->dev, "register VBUS failed\n");
return ret;
}
@@ -795,20 +793,6 @@ static int ci_extcon_register(struct ci_hdrc *ci)
return 0;
 }
 
-static void ci_extcon_unregister(struct ci_hdrc *ci)
-{
-   struct ci_hdrc_cable *cable;
-
-   cable = &ci->platdata->id_extcon;
-   if (!IS_ERR(cable->edev))
-   extcon_unregister_notifier(cable->edev, EXTCON_USB_HOST,
-  &cable->nb);
-
-   cable = &ci->platdata->vbus_extcon;
-   if (!IS_ERR(cable->edev))
-   extcon_unregister_notifier(cable->edev, EXTCON_USB, &cable->nb);
-}
-
 static DEFINE_IDA(ci_ida);
 
 struct platform_device *ci_hdrc_add_device(struct device *dev,
@@ -1053,7 +1037,6 @@ static int ci_hdrc_probe(struct platform_device *pdev)
if (!ret)
return 0;
 
-   ci_extcon_unregister(ci);
 stop:
ci_role_destroy(ci);
 deinit_phy:
@@ -1073,7 +1056,6 @@ static int ci_hdrc_remove(struct platform_device *pdev)
}
 
dbg_remove_files(ci);
-   ci_extcon_unregister(ci);
ci_role_destroy(ci);
ci_hdrc_enter_lpm(ci, true);
ci_usb_phy_exit(ci);
-- 
1.9.1



[PATCH 07/12] usb: sunxi: Uses the resource-managed extcon API when registering extcon notifier

2016-11-29 Thread Chanwoo Choi
This patch just uses the resource-managed extcon API when registering
the extcon notifier.

Signed-off-by: Chanwoo Choi 
---
 drivers/usb/musb/sunxi.c | 12 +++-
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/usb/musb/sunxi.c b/drivers/usb/musb/sunxi.c
index 1408245be18e..4b531551e49d 100644
--- a/drivers/usb/musb/sunxi.c
+++ b/drivers/usb/musb/sunxi.c
@@ -261,14 +261,14 @@ static int sunxi_musb_init(struct musb *musb)
writeb(SUNXI_MUSB_VEND0_PIO_MODE, musb->mregs + SUNXI_MUSB_VEND0);
 
/* Register notifier before calling phy_init() */
-   ret = extcon_register_notifier(glue->extcon, EXTCON_USB_HOST,
-  &glue->host_nb);
+   ret = devm_extcon_register_notifier(glue->dev, glue->extcon,
+   EXTCON_USB_HOST, &glue->host_nb);
if (ret)
goto error_reset_assert;
 
ret = phy_init(glue->phy);
if (ret)
-   goto error_unregister_notifier;
+   goto error_reset_assert;
 
musb->isr = sunxi_musb_interrupt;
 
@@ -277,9 +277,6 @@ static int sunxi_musb_init(struct musb *musb)
 
return 0;
 
-error_unregister_notifier:
-   extcon_unregister_notifier(glue->extcon, EXTCON_USB_HOST,
-  &glue->host_nb);
 error_reset_assert:
if (test_bit(SUNXI_MUSB_FL_HAS_RESET, &glue->flags))
reset_control_assert(glue->rst);
@@ -303,9 +300,6 @@ static int sunxi_musb_exit(struct musb *musb)
 
phy_exit(glue->phy);
 
-   extcon_unregister_notifier(glue->extcon, EXTCON_USB_HOST,
-  &glue->host_nb);
-
if (test_bit(SUNXI_MUSB_FL_HAS_RESET, &glue->flags))
reset_control_assert(glue->rst);
 
-- 
1.9.1



[PATCH 03/12] power_supply: axp288_charger: Replace the extcon API

2016-11-29 Thread Chanwoo Choi
This patch uses the resource-managed extcon API for extcon_register_notifier()
and replaces the deprecated extcon API as following:
- extcon_get_cable_state_() -> extcon_get_state()

Signed-off-by: Chanwoo Choi 
---
 drivers/power/supply/axp288_charger.c | 51 +--
 1 file changed, 13 insertions(+), 38 deletions(-)

diff --git a/drivers/power/supply/axp288_charger.c 
b/drivers/power/supply/axp288_charger.c
index 75b8e0c7402b..1115052e9a69 100644
--- a/drivers/power/supply/axp288_charger.c
+++ b/drivers/power/supply/axp288_charger.c
@@ -581,15 +581,15 @@ static void axp288_charger_extcon_evt_worker(struct 
work_struct *work)
bool old_connected = info->cable.connected;
 
/* Determine cable/charger type */
-   if (extcon_get_cable_state_(edev, EXTCON_CHG_USB_SDP) > 0) {
+   if (extcon_get_state(edev, EXTCON_CHG_USB_SDP) > 0) {
dev_dbg(&info->pdev->dev, "USB SDP charger  is connected");
info->cable.connected = true;
info->cable.chg_type = POWER_SUPPLY_TYPE_USB;
-   } else if (extcon_get_cable_state_(edev, EXTCON_CHG_USB_CDP) > 0) {
+   } else if (extcon_get_state(edev, EXTCON_CHG_USB_CDP) > 0) {
dev_dbg(&info->pdev->dev, "USB CDP charger is connected");
info->cable.connected = true;
info->cable.chg_type = POWER_SUPPLY_TYPE_USB_CDP;
-   } else if (extcon_get_cable_state_(edev, EXTCON_CHG_USB_DCP) > 0) {
+   } else if (extcon_get_state(edev, EXTCON_CHG_USB_DCP) > 0) {
dev_dbg(&info->pdev->dev, "USB DCP charger is connected");
info->cable.connected = true;
info->cable.chg_type = POWER_SUPPLY_TYPE_USB_DCP;
@@ -686,7 +686,7 @@ static int axp288_charger_handle_otg_evt(struct 
notifier_block *nb,
struct axp288_chrg_info *info =
container_of(nb, struct axp288_chrg_info, otg.id_nb);
struct extcon_dev *edev = info->otg.cable;
-   int usb_host = extcon_get_cable_state_(edev, EXTCON_USB_HOST);
+   int usb_host = extcon_get_state(edev, EXTCON_USB_HOST);
 
dev_dbg(&info->pdev->dev, "external connector USB-Host is %s\n",
usb_host ? "attached" : "detached");
@@ -841,33 +841,27 @@ static int axp288_charger_probe(struct platform_device 
*pdev)
/* Register for extcon notification */
INIT_WORK(&info->cable.work, axp288_charger_extcon_evt_worker);
info->cable.nb.notifier_call = axp288_charger_handle_cable_evt;
-   ret = extcon_register_notifier(info->cable.edev, EXTCON_CHG_USB_SDP,
-   &info->cable.nb);
+   ret = devm_extcon_register_notifier(&pdev->dev, info->cable.edev,
+   EXTCON_CHG_USB_SDP, &info->cable.nb);
if (ret) {
dev_err(&info->pdev->dev,
"failed to register extcon notifier for SDP %d\n", ret);
return ret;
}
 
-   ret = extcon_register_notifier(info->cable.edev, EXTCON_CHG_USB_CDP,
-   &info->cable.nb);
+   ret = devm_extcon_register_notifier(&pdev->dev, info->cable.edev,
+   EXTCON_CHG_USB_CDP, &info->cable.nb);
if (ret) {
dev_err(&info->pdev->dev,
"failed to register extcon notifier for CDP %d\n", ret);
-   extcon_unregister_notifier(info->cable.edev,
-   EXTCON_CHG_USB_SDP, &info->cable.nb);
return ret;
}
 
-   ret = extcon_register_notifier(info->cable.edev, EXTCON_CHG_USB_DCP,
-   &info->cable.nb);
+   ret = devm_extcon_register_notifier(&pdev->dev, info->cable.edev,
+   EXTCON_CHG_USB_DCP, &info->cable.nb);
if (ret) {
dev_err(&info->pdev->dev,
"failed to register extcon notifier for DCP %d\n", ret);
-   extcon_unregister_notifier(info->cable.edev,
-   EXTCON_CHG_USB_SDP, &info->cable.nb);
-   extcon_unregister_notifier(info->cable.edev,
-   EXTCON_CHG_USB_CDP, &info->cable.nb);
return ret;
}
 
@@ -887,13 +881,13 @@ static int axp288_charger_probe(struct platform_device 
*pdev)
/* Register for OTG notification */
INIT_WORK(&info->otg.work, axp288_charger_otg_evt_worker);
info->otg.id_nb.notifier_call = axp288_charger_handle_otg_evt;
-   ret = extcon_register_notifier(info->otg.cable, EXTCON_USB_HOST,
-  &info->otg.id_nb);
+   ret = devm_extcon_register_notifier(&pdev->dev, info->otg.cable,
+   EXTCON_USB_HOST, &info->otg.id_nb);
if (ret)
dev_warn(&pdev->dev, "failed to register otg notifier\n");
 
if (info->o

[PATCH 11/12] usb: phy: tahvo: Replace the deprecated extcon API

2016-11-29 Thread Chanwoo Choi
This patch replaces the deprecated extcon API as following:
- extcon_set_cable_state_() -> extcon_set_state_sync()

Signed-off-by: Chanwoo Choi 
---
 drivers/usb/phy/phy-tahvo.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/phy/phy-tahvo.c b/drivers/usb/phy/phy-tahvo.c
index 1343bff004eb..9a7822e12156 100644
--- a/drivers/usb/phy/phy-tahvo.c
+++ b/drivers/usb/phy/phy-tahvo.c
@@ -121,7 +121,7 @@ static void check_vbus_state(struct tahvo_usb *tu)
prev_state = tu->vbus_state;
tu->vbus_state = reg & TAHVO_STAT_VBUS;
if (prev_state != tu->vbus_state) {
-   extcon_set_cable_state_(tu->extcon, EXTCON_USB, tu->vbus_state);
+   extcon_set_state_sync(tu->extcon, EXTCON_USB, tu->vbus_state);
sysfs_notify(&tu->pt_dev->dev.kobj, NULL, "vbus_state");
}
 }
@@ -130,7 +130,7 @@ static void tahvo_usb_become_host(struct tahvo_usb *tu)
 {
struct retu_dev *rdev = dev_get_drvdata(tu->pt_dev->dev.parent);
 
-   extcon_set_cable_state_(tu->extcon, EXTCON_USB_HOST, true);
+   extcon_set_state_sync(tu->extcon, EXTCON_USB_HOST, true);
 
/* Power up the transceiver in USB host mode */
retu_write(rdev, TAHVO_REG_USBR, USBR_REGOUT | USBR_NSUSPEND |
@@ -149,7 +149,7 @@ static void tahvo_usb_become_peripheral(struct tahvo_usb 
*tu)
 {
struct retu_dev *rdev = dev_get_drvdata(tu->pt_dev->dev.parent);
 
-   extcon_set_cable_state_(tu->extcon, EXTCON_USB_HOST, false);
+   extcon_set_state_sync(tu->extcon, EXTCON_USB_HOST, false);
 
/* Power up transceiver and set it in USB peripheral mode */
retu_write(rdev, TAHVO_REG_USBR, USBR_SLAVE_CONTROL | USBR_REGOUT |
@@ -379,9 +379,9 @@ static int tahvo_usb_probe(struct platform_device *pdev)
}
 
/* Set the initial cable state. */
-   extcon_set_cable_state_(tu->extcon, EXTCON_USB_HOST,
+   extcon_set_state_sync(tu->extcon, EXTCON_USB_HOST,
   tu->tahvo_mode == TAHVO_MODE_HOST);
-   extcon_set_cable_state_(tu->extcon, EXTCON_USB, tu->vbus_state);
+   extcon_set_state_sync(tu->extcon, EXTCON_USB, tu->vbus_state);
 
/* Create OTG interface */
tahvo_usb_power_off(tu);
-- 
1.9.1



[PATCH 00/12] extcon: Replace the deprecated extcon API

2016-11-29 Thread Chanwoo Choi
This patches just replace the deprecated extcon API without any change
of extcon operation and use the resource-managed function for
extcon_register_notifier().

The new extcon API instead of deprecated API.
- extcon_set_cable_state_() -> extcon_set_state_sync();
- extcon_get_cable_state_() -> extcon_get_state();

The each patch has not any dependency among patches. So, each maintainer
could pick up each patch without any problem.

Chanwoo Choi (12):
  phy: rcar-gen3-usb2: Replace the deprecated extcon API
  phy: sun4i-usb: Replace the deprecated extcon API
  power_supply: axp288_charger: Replace the extcon API
  power_supply: qcom_smbb: Replace the deprecated extcon API
  usb: chipdata: Replace the extcon API
  usb: dwc3: omap: Replace the extcon API
  usb: sunxi: Uses the resource-managed extcon API when registering extcon 
notifier
  usb: phy: msm: Replace the extcon API
  usb: phy: omap-otg: Replace the extcon API
  usb: phy: qcom-8x16-usb: Replace the extcon API
  usb: phy: tahvo: Replace the deprecated extcon API
  usb: renesas_usbhs: Replace the deprecated extcon API

 drivers/phy/phy-rcar-gen3-usb2.c  |  8 +++---
 drivers/phy/phy-sun4i-usb.c   |  4 +--
 drivers/power/supply/axp288_charger.c | 51 +--
 drivers/power/supply/qcom_smbb.c  |  2 +-
 drivers/usb/chipidea/core.c   | 30 +
 drivers/usb/dwc3/dwc3-omap.c  | 20 +-
 drivers/usb/musb/sunxi.c  | 12 +++--
 drivers/usb/phy/phy-msm-usb.c | 33 ---
 drivers/usb/phy/phy-omap-otg.c| 24 +
 drivers/usb/phy/phy-qcom-8x16-usb.c   | 13 +++--
 drivers/usb/phy/phy-tahvo.c   | 10 +++
 drivers/usb/renesas_usbhs/common.c|  2 +-
 12 files changed, 63 insertions(+), 146 deletions(-)

-- 
1.9.1



[PATCH 01/12] phy: rcar-gen3-usb2: Replace the deprecated extcon API

2016-11-29 Thread Chanwoo Choi
This patch replaces the deprecated extcon API as following:
- extcon_set_cable_state_() -> extcon_set_state_sync()

Signed-off-by: Chanwoo Choi 
---
 drivers/phy/phy-rcar-gen3-usb2.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/phy/phy-rcar-gen3-usb2.c b/drivers/phy/phy-rcar-gen3-usb2.c
index bd2430d7339c..7f8081f157f4 100644
--- a/drivers/phy/phy-rcar-gen3-usb2.c
+++ b/drivers/phy/phy-rcar-gen3-usb2.c
@@ -93,11 +93,11 @@ static void rcar_gen3_phy_usb2_work(struct work_struct 
*work)
 work);
 
if (ch->extcon_host) {
-   extcon_set_cable_state_(ch->extcon, EXTCON_USB_HOST, true);
-   extcon_set_cable_state_(ch->extcon, EXTCON_USB, false);
+   extcon_set_state_sync(ch->extcon, EXTCON_USB_HOST, true);
+   extcon_set_state_sync(ch->extcon, EXTCON_USB, false);
} else {
-   extcon_set_cable_state_(ch->extcon, EXTCON_USB_HOST, false);
-   extcon_set_cable_state_(ch->extcon, EXTCON_USB, true);
+   extcon_set_state_sync(ch->extcon, EXTCON_USB_HOST, false);
+   extcon_set_state_sync(ch->extcon, EXTCON_USB, true);
}
 }
 
-- 
1.9.1



Re: [PATCH v3 2/4] thermal: rcar_gen3_thermal: Add R-Car Gen3 thermal driver

2016-11-29 Thread Eduardo Valentin
Hey,

On Tue, Nov 29, 2016 at 09:32:42AM +0100, Wolfram Sang wrote:
> Hi Eduardo,
> 
> > No commit message ? :-( generally speaking here, it is a good practice
> > to describe what you are doing, what you have considered, and what you
> > have tested, just for the sake of code history/documentation.
> 
> OK, can do.

cool!

> 
> > > + spin_lock_irqsave(&tsc->lock, flags);
> > 
> > Why do you need a full blown spin lock irqsave? Can this locking be
> > solved by a simple mutex?
> 
> Currently, it can be a mutex, yes. This is a "left-over" from the
> version which had interrupt support. I am not fully done with
> refactoring the irqs, but I thought it is likely we need this lock then
> again, so I chose to leave it. I can use a mutex for now if you prefer.

yes, yield the processor when possible, please. So, if your locking
don't really need to disable IRQs, then avoid doing it. If a mutex is
enough, use it.

> 
> > 
> > > +static void r8a7795_thermal_init(struct rcar_gen3_thermal_tsc *tsc)
> > > +{
> > > + unsigned long flags;
> > > +
> > > + spin_lock_irqsave(&tsc->lock, flags);
> > > +
> > > + rcar_gen3_thermal_write(tsc, REG_GEN3_CTSR,  CTSR_THBGR);
> > > + rcar_gen3_thermal_write(tsc, REG_GEN3_CTSR,  0x0);
> > > +
> > > + udelay(1000);
> > 
> > Why do you disable irqs, then busyloop for 1ms?
> 
> Uh yes, this is ugly. I don't think we need to lock during init, but
> I'll double check later.
> 

OK. You could probably leave the (mutex) lock, if you think you need it.
But also consider using usleep_range(1000) instead. Have a look at:
Documentation/timers/timers-howto.txt

for a better explanation.


> > > + /* default values if FUSEs are missing */
> > > + int ptat[3] = { 2351, 1509, 435 };
> > > + int thcode[TSC_MAX_NUM][3] = {
> > > + { 3248, 2800, 2221 },
> > > + { 3245, 2795, 2216 },
> > > + { 3250, 2805, 2237 },
> > > + };
> > 
> > are these fuses valid for both? 7796 and 7797? or would you need a
> > different array for each version?
> 
> According to the information I have today, it is the same array. And all
> later versions are promised to have fuse registers. This is already
> documented, but such HW is not available to me currently.
> 

Ok. If you have the confirmation for that, then it is fine. I just asked
because cases I saw (different manufacturer of course) would have different 
values
to be programmed, depending on the chip version.

Anyways, just checking.

> Thanks for the quick review, very much appreciated!
> 
>Wolfram
> 




Re: [PATCH v3 03/13] drm: bridge: Link encoder and bridge in core code

2016-11-29 Thread Archit Taneja



On 11/29/2016 11:27 PM, Laurent Pinchart wrote:

Hi Archit,

On Tuesday 29 Nov 2016 15:57:06 Archit Taneja wrote:

On 11/29/2016 02:34 PM, Laurent Pinchart wrote:

Instead of linking encoders and bridges in every driver (and getting it
wrong half of the time, as many drivers forget to set the drm_bridge
encoder pointer), do so in core code. The drm_bridge_attach() function
needs the encoder and optional previous bridge to perform that task,
update all the callers.

Signed-off-by: Laurent Pinchart

---

 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c   |  4 +-
 drivers/gpu/drm/bridge/analogix/analogix_dp_core.c |  4 +-
 drivers/gpu/drm/bridge/dw-hdmi.c   |  3 +-
 drivers/gpu/drm/drm_bridge.c   | 46 -
 drivers/gpu/drm/drm_simple_kms_helper.c|  4 +-
 drivers/gpu/drm/exynos/exynos_dp.c |  5 +--
 drivers/gpu/drm/exynos/exynos_drm_dsi.c|  6 +--
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c  |  5 +--
 drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c   |  5 +--
 drivers/gpu/drm/imx/imx-ldb.c  |  6 +--
 drivers/gpu/drm/imx/parallel-display.c |  4 +-
 drivers/gpu/drm/mediatek/mtk_dpi.c |  8 ++--
 drivers/gpu/drm/mediatek/mtk_dsi.c | 24 ++-
 drivers/gpu/drm/mediatek/mtk_hdmi.c| 11 +++---
 drivers/gpu/drm/msm/dsi/dsi_manager.c  | 17 +---
 drivers/gpu/drm/msm/edp/edp_bridge.c   |  2 +-
 drivers/gpu/drm/msm/hdmi/hdmi_bridge.c |  2 +-
 drivers/gpu/drm/rcar-du/rcar_du_hdmienc.c  |  5 +--
 drivers/gpu/drm/sti/sti_dvo.c  |  3 +-
 drivers/gpu/drm/sti/sti_hda.c  |  3 +-
 drivers/gpu/drm/sti/sti_hdmi.c |  3 +-
 drivers/gpu/drm/sun4i/sun4i_rgb.c  | 13 +++---
 include/drm/drm_bridge.h   |  3 +-
 23 files changed, 83 insertions(+), 103 deletions(-)


[snip]


diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
index 0ee052b7c21a..850bd6509ef1 100644
--- a/drivers/gpu/drm/drm_bridge.c
+++ b/drivers/gpu/drm/drm_bridge.c


[snip]


@@ -92,32 +93,53 @@ void drm_bridge_remove(struct drm_bridge *bridge)
 EXPORT_SYMBOL(drm_bridge_remove);

 /**
- * drm_bridge_attach - associate given bridge to our DRM device
+ * drm_bridge_attach - attach the bridge to an encoder's chain
  *
- * @dev: DRM device
- * @bridge: bridge control structure
+ * @encoder: DRM encoder
+ * @bridge: bridge to attach
+ * @previous: previous bridge in the chain (optional)
  *
- * Called by a kms driver to link one of our encoder/bridge to the given
- * bridge.
+ * Called by a kms driver to link the bridge to an encoder's chain. The
previous
+ * argument specifies the previous bridge in the chain. If NULL, the
bridge is
+ * linked directly at the encoder's output. Otherwise it is linked at the
+ * previous bridge's output.
  *
- * Note that setting up links between the bridge and our encoder/bridge
- * objects needs to be handled by the kms driver itself.
+ * If non-NULL the previous bridge must be already attached by a call to
this
+ * function.
  *
  * RETURNS:
  * Zero on success, error code on failure
  */
-int drm_bridge_attach(struct drm_device *dev, struct drm_bridge *bridge)
+int drm_bridge_attach(struct drm_encoder *encoder, struct drm_bridge
*bridge,
+ struct drm_bridge *previous)
 {
-   if (!dev || !bridge)
+   int ret;
+
+   if (!encoder || !bridge)
+   return -EINVAL;


I think we could derive previous from the encoder itself. Something like:

previous = encoder->bridge;
while (previous && previous->next)
previous = previous->next;


That's a very good point. It would however prevent us from catching drivers
that attach bridges in the wrong order, which the !previous->dev currently
allows us to do (and it should be turned into a WARN_ON as Daniel proposed).



With the simpler API, I don't think we will ever hit the case of
!previous->dev. The previous bridge (if it exists) in the chain would
already have a dev attached to it. In other words, we would remove the
risk of the chance of the 'previous' bridge being unattached.

I'm a bit unclear about what you mean about the order part. If a kms driver
wants to create a chain: encoder->bridge1->bridge2, it should ideally do:

drm_bridge_attach(encoder, bridge1, NULL);
drm_bridge_attach(encoder, bridge2, bridge1);

We can't do much if the kms driver does the opposite:

drm_bridge_attach(encoder, bridge2, NULL);
drm_bridge_attach(encoder, bridge2, bridge1);



I'm fine losing that ability, as your proposal makes the API simpler. I'll let
you decide, which option do you prefer ?


I prefer the simpler API. I guess the main aim of the patch was to prevent the
driver setting up the encoder<->bridge links, which will be done anyway.

Thanks,
Archit




+
+   if (previous && (!previous

Re: [PATCH net 00/16] net: fix fixed-link phydev leaks

2016-11-29 Thread David Miller
From: Johan Hovold 
Date: Mon, 28 Nov 2016 19:24:53 +0100

> This series fixes failures to deregister and free fixed-link phydevs
> that have been registered using the of_phy_register_fixed_link()
> interface.
> 
> All but two drivers currently fail to do this and this series fixes most
> of them with the exception of a staging driver and the stmmac drivers
> which will be fixed by follow-on patches.
> 
> Included are also a couple of fixes for related of-node leaks.
> 
> Note that all patches except the of_mdio one have been compile-tested
> only.
> 
> Also note that the series is against net due to dependencies not yet in
> net-next.

Series applied, thanks Johan.


Re: [PATCH v2] net: phy: Fix use after free in phy_detach()

2016-11-29 Thread David Miller
From: Geert Uytterhoeven 
Date: Mon, 28 Nov 2016 15:18:31 +0100

> If device_release_driver(&phydev->mdio.dev) is called, it releases all
> resources belonging to the PHY device. Hence the subsequent call to
> phy_led_triggers_unregister() will access already freed memory when
> unregistering the LEDs.
> 
> Move the call to phy_led_triggers_unregister() before the possible call
> to device_release_driver() to fix this.
> 
> Fixes: 2e0bc452f4721520 ("net: phy: leds: add support for led triggers on phy 
> link state change")
> Signed-off-by: Geert Uytterhoeven 
> ---
> This is v2 of "[RFC] net: phy: Fix double free in phy_detach()".
> 
> v2:
>   - Dropped RFC,
>   - Reworded, as commit a7dac9f9c1695d74 ("phy: fix error case of
> phy_led_triggers_(un)register") fixed the double free, and thus the
> warning I was seeing during "poweroff" on sh73a0/kzm9g,
>   - Verified use after free using CONFIG_DEBUG_DEVRES, log_devres = 1,
> and additional debug code printing the address of
> phy->phy_led_triggers. Adding poisoning of freed memory to
> devres_log() will cause a crash.

Applied, thanks.


Re: [PATCH v3 09/13] drm: Add encoder_type field to the drm_bridge structure

2016-11-29 Thread Laurent Pinchart
Hi Daniel,

On Tuesday 29 Nov 2016 21:25:27 Daniel Vetter wrote:
> On Tue, Nov 29, 2016 at 07:49:22PM +0200, Laurent Pinchart wrote:
> > On Tuesday 29 Nov 2016 11:27:20 Daniel Vetter wrote:
> >> On Tue, Nov 29, 2016 at 11:58:44AM +0200, Laurent Pinchart wrote:
> >>> On Tuesday 29 Nov 2016 10:56:53 Daniel Vetter wrote:
>  On Tue, Nov 29, 2016 at 11:04:39AM +0200, Laurent Pinchart wrote:
> > The drm_bridge object models on- or off-chip hardware encoders and
> > provide an abstract control API to display drivers. In order to help
> > display drivers creating the right kind of drm_encoder object,
> > expose the type of the hardware encoder associated with each bridge.
> > 
> > Signed-off-by: Laurent Pinchart
> > 
>  
>  DRM_MODE_ENCODER_BRIDGE. Problem solved, because in reality no one
>  cares one iota about the encoder type.
> >>> 
> >>> It's exposed to userspace though, are you 100% sure we won't break
> >>> anything ?
> >> 
> >> We've added DP, DSI, DPMST and DPI encoder types thus far, no one
> >> screamed.
> > 
> > In that case why don't we go one step further and remove the encoder type
> > completely ? We can't remove the field from the API, but we can hardcode
> > it to a single value.
> > 
> > There are however drivers that rely on the encoder type (radeon, nouveau,
> > sti, amdgpu, msm and rcar-du, but I'll fix the last one) so we'd need to
> > address that first. If we don't want to remove the encoder_type field
> > from in-kernel structures and let drivers use it, then I don't think
> > DRM_MODE_ENCODER_BRIDGE would be a good option, we should report the real
> > type instead.
> 
> If you strongly believe that I will not stop you. This was just a
> suggestion to get all your stuff landed with minimal amounts of effort and
> across-the-subsystem cleanup needed. I'd do it that ;-)
> 
> And if you don't like DRM_MODE_ENCODER_BRIDGE you could also pick
> DRM_MODE_ENCODER_NONE, which is what most seem to do today. In the end it
> doesn't matter no matter which option you pick. The only difference is in
> the amount of effort you need to spend to get it merged ...

I've tried hardcoding the encoder type to DRM_MODE_ENCODER_NONE and basic 
tests still pass (I haven't tried more complex userspace stacks such as X or 
Weston though). I'll thus drop the addition of encoder_type to the bridge for 
now. As a result we should start deprecating the drm_encoder::encoder_type 
field (unless a compelling reason is found of course, in which case I'd have 
to revive drm_bridge::encoder_type).

-- 
Regards,

Laurent Pinchart



Re: [PATCH] pinctrl: sh-pfc: r8a7791: Add ADI pinconf support

2016-11-29 Thread Geert Uytterhoeven
Hi Laurent,

On Tue, Nov 29, 2016 at 8:30 PM, Laurent Pinchart
 wrote:
> On Tuesday 29 Nov 2016 10:11:56 Jacopo Mondi wrote:
>> Add pin configuration support for Gyro-ADC, named ADI on r8a7791 SoC.
>>
>> The Gyro-ADC supports three different configurations:
>> a single ADC (adi and adi_b groups), 2 ADCs selectable through a single
>> channel select signal (adi_chsel1 and adi_chsel1_b groups),
>
> I've only briefly looked up at the datasheet, but is that a supported mode ?
> It seems that mode 1 uses 4 channels and mode 2 and 3 use 8 channels.

I think you can always connect less ADCs than there are channels.
If you connect e.g. only one, you don't need any CHS signals.
If you connect e.g. two, you can use only one CHS signal.

AFAICS, you still have to use an external demux to create individual chip
selects from the single CS signal and (up to 3) CHS signals.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds


Re: [PATCH v3 06/13] drm: bridge: Add LVDS encoder driver

2016-11-29 Thread Laurent Pinchart
Hi Daniel,

On Tuesday 29 Nov 2016 10:54:09 Daniel Vetter wrote:
> On Tue, Nov 29, 2016 at 11:04:36AM +0200, Laurent Pinchart wrote:
> > The LVDS encoder driver is a DRM bridge driver that supports the
> > parallel to LVDS encoders that don't require any configuration. The
> > driver thus doesn't interact with the device, but creates an LVDS
> > connector for the panel and exposes its size and timing based on
> > information retrieved from DT.
> > 
> > Signed-off-by: Laurent Pinchart
> > 
> 
> Since it's 100% dummy, why put LVDS into the name? This little thing here
> could be our generic "wrap drm_panel and attach it to a chain" helper. So
> what about calling this _The_ drm_panel_bridge, and also linking it into
> docs to feature it a bit more prominently.

I'm not opposed to that, except that this driver should not be considered as 
just a helper to link a panel. It should only be used to support a real 
hardware bridge that requires no control.

> I came up with this because I spotted some refactoring belows for building
> this helper, until I realized that this driver _is_ the helper I think we
> want ;-) Only thing missing is an exported function to instantiate a
> bridge with just a drm_panel as the parameter. And putting it into the
> drm_kms_helper.ko module.

What would that be used for ? The bridge should be instantiated by this bridge 
driver, bound to a bridge device instantiated from DT (or the platform 
firmware of your choice).

> > +static enum drm_connector_status
> > +lvds_connector_detect(struct drm_connector *connector, bool force)
> > +{
> > +   return connector_status_connected;
> > +}
> 
> We have piles of this exact dummy callback all over, maybe make it the
> default and rip them all out?

Done, "[PATCH] drm: Make the connector .detect() callback optional".

-- 
Regards,

Laurent Pinchart



Re: [PATCH v3 09/13] drm: Add encoder_type field to the drm_bridge structure

2016-11-29 Thread Daniel Vetter
On Tue, Nov 29, 2016 at 07:49:22PM +0200, Laurent Pinchart wrote:
> Hi Daniel,
> 
> On Tuesday 29 Nov 2016 11:27:20 Daniel Vetter wrote:
> > On Tue, Nov 29, 2016 at 11:58:44AM +0200, Laurent Pinchart wrote:
> > > On Tuesday 29 Nov 2016 10:56:53 Daniel Vetter wrote:
> > >> On Tue, Nov 29, 2016 at 11:04:39AM +0200, Laurent Pinchart wrote:
> > >>> The drm_bridge object models on- or off-chip hardware encoders and
> > >>> provide an abstract control API to display drivers. In order to help
> > >>> display drivers creating the right kind of drm_encoder object, expose
> > >>> the type of the hardware encoder associated with each bridge.
> > >>> 
> > >>> Signed-off-by: Laurent Pinchart
> > >>> 
> > >> 
> > >> DRM_MODE_ENCODER_BRIDGE. Problem solved, because in reality no one cares
> > >> one iota about the encoder type.
> > > 
> > > It's exposed to userspace though, are you 100% sure we won't break
> > > anything ?
> >
> > We've added DP, DSI, DPMST and DPI encoder types thus far, no one
> > screamed.
> 
> In that case why don't we go one step further and remove the encoder type 
> completely ? We can't remove the field from the API, but we can hardcode it 
> to 
> a single value.
> 
> There are however drivers that rely on the encoder type (radeon, nouveau, 
> sti, 
> amdgpu, msm and rcar-du, but I'll fix the last one) so we'd need to address 
> that first. If we don't want to remove the encoder_type field from in-kernel 
> structures and let drivers use it, then I don't think DRM_MODE_ENCODER_BRIDGE 
> would be a good option, we should report the real type instead.

If you strongly believe that I will not stop you. This was just a
suggestion to get all your stuff landed with minimal amounts of effort and
across-the-subsystem cleanup needed. I'd do it that ;-)

And if you don't like DRM_MODE_ENCODER_BRIDGE you could also pick
DRM_MODE_ENCODER_NONE, which is what most seem to do today. In the end it
doesn't matter no matter which option you pick. The only difference is in
the amount of effort you need to spend to get it merged ...
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch


Re: [PATCH v3 04/13] drm: bridge: Detach bridge from encoder at encoder cleanup time

2016-11-29 Thread Daniel Vetter
On Tue, Nov 29, 2016 at 08:56:44PM +0200, Laurent Pinchart wrote:
> Hi Archit,
> 
> On Tuesday 29 Nov 2016 16:04:08 Archit Taneja wrote:
> > On 11/29/2016 02:34 PM, Laurent Pinchart wrote:
> > > Most drivers that use bridges forgot to detach them at cleanup time.
> > > Instead of fixing them one by one, detach the bridge in the core
> > > drm_encoder_cleanup() function.
> > > 
> > > Signed-off-by: Laurent Pinchart
> > > 
> > > ---
> > > 
> > >  drivers/gpu/drm/drm_encoder.c | 3 +++
> > >  1 file changed, 3 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/drm_encoder.c b/drivers/gpu/drm/drm_encoder.c
> > > index 5c067719164d..9c1f99646e0d 100644
> > > --- a/drivers/gpu/drm/drm_encoder.c
> > > +++ b/drivers/gpu/drm/drm_encoder.c
> > > @@ -164,6 +164,9 @@ void drm_encoder_cleanup(struct drm_encoder *encoder)
> > >* the indices on the drm_encoder after us in the encoder_list.
> > >*/
> > > + if (encoder->bridge)
> > > + drm_bridge_detach(encoder->bridge);
> > 
> > This would require the kms driver to still detach the remaining
> > n - 1 bridges in a possible chain. We could probably detach all of
> > them here, or maybe leave detaching of all to the kms driver, and just
> > report a warning here.
> 
> I'd prefer detaching them all here, but that's a bit intrusive and should be 
> tested correctly. The patch series is already growing big, could we do that 
> in 
> a separate patch ?

I think you're bridge-for-panels driver is the first one that's actually
getting chained. I guess you do have to fix this here ;-) And if you go
through with making drm_bridge_detach be a purely drm.ko internal thing,
we can make it recursive like all the other functions. Problem solved.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch


Re: [PATCH v3 03/13] drm: bridge: Link encoder and bridge in core code

2016-11-29 Thread Boris Brezillon
On Tue, 29 Nov 2016 11:04:33 +0200
Laurent Pinchart  wrote:

> Instead of linking encoders and bridges in every driver (and getting it
> wrong half of the time, as many drivers forget to set the drm_bridge
> encoder pointer), do so in core code. The drm_bridge_attach() function
> needs the encoder and optional previous bridge to perform that task,
> update all the callers.
> 
> Signed-off-by: Laurent Pinchart 
> ---
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c   |  4 +-

For atmel-hlcdc

Acked-by: Boris Brezillon 

>  drivers/gpu/drm/bridge/analogix/analogix_dp_core.c |  4 +-
>  drivers/gpu/drm/bridge/dw-hdmi.c   |  3 +-
>  drivers/gpu/drm/drm_bridge.c   | 46 
> --
>  drivers/gpu/drm/drm_simple_kms_helper.c|  4 +-
>  drivers/gpu/drm/exynos/exynos_dp.c |  5 +--
>  drivers/gpu/drm/exynos/exynos_drm_dsi.c|  6 +--
>  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c  |  5 +--
>  drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c   |  5 +--
>  drivers/gpu/drm/imx/imx-ldb.c  |  6 +--
>  drivers/gpu/drm/imx/parallel-display.c |  4 +-
>  drivers/gpu/drm/mediatek/mtk_dpi.c |  8 ++--
>  drivers/gpu/drm/mediatek/mtk_dsi.c | 24 ++-
>  drivers/gpu/drm/mediatek/mtk_hdmi.c| 11 +++---
>  drivers/gpu/drm/msm/dsi/dsi_manager.c  | 17 +---
>  drivers/gpu/drm/msm/edp/edp_bridge.c   |  2 +-
>  drivers/gpu/drm/msm/hdmi/hdmi_bridge.c |  2 +-
>  drivers/gpu/drm/rcar-du/rcar_du_hdmienc.c  |  5 +--
>  drivers/gpu/drm/sti/sti_dvo.c  |  3 +-
>  drivers/gpu/drm/sti/sti_hda.c  |  3 +-
>  drivers/gpu/drm/sti/sti_hdmi.c |  3 +-
>  drivers/gpu/drm/sun4i/sun4i_rgb.c  | 13 +++---
>  include/drm/drm_bridge.h   |  3 +-
>  23 files changed, 83 insertions(+), 103 deletions(-)
> 
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c 
> b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c
> index 6119b5085501..e7799b6ee829 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c
> @@ -230,9 +230,7 @@ static int atmel_hlcdc_attach_endpoint(struct drm_device 
> *dev,
>   of_node_put(np);
>  
>   if (bridge) {
> - output->encoder.bridge = bridge;
> - bridge->encoder = &output->encoder;
> - ret = drm_bridge_attach(dev, bridge);
> + ret = drm_bridge_attach(&output->encoder, bridge, NULL);
>   if (!ret)
>   return 0;
>   }
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c 
> b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> index 6e0447f329a2..1835f1fdad19 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> @@ -1227,12 +1227,10 @@ static int analogix_dp_create_bridge(struct 
> drm_device *drm_dev,
>  
>   dp->bridge = bridge;
>  
> - dp->encoder->bridge = bridge;
>   bridge->driver_private = dp;
> - bridge->encoder = dp->encoder;
>   bridge->funcs = &analogix_dp_bridge_funcs;
>  
> - ret = drm_bridge_attach(drm_dev, bridge);
> + ret = drm_bridge_attach(dp->encoder, bridge, NULL);
>   if (ret) {
>   DRM_ERROR("failed to attach drm bridge\n");
>   return -EINVAL;
> diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c 
> b/drivers/gpu/drm/bridge/dw-hdmi.c
> index b71088dab268..432e0e3fff72 100644
> --- a/drivers/gpu/drm/bridge/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/dw-hdmi.c
> @@ -1841,13 +1841,12 @@ static int dw_hdmi_register(struct drm_device *drm, 
> struct dw_hdmi *hdmi)
>   hdmi->bridge = bridge;
>   bridge->driver_private = hdmi;
>   bridge->funcs = &dw_hdmi_bridge_funcs;
> - ret = drm_bridge_attach(drm, bridge);
> + ret = drm_bridge_attach(encoder, bridge, NULL);
>   if (ret) {
>   DRM_ERROR("Failed to initialize bridge with drm\n");
>   return -EINVAL;
>   }
>  
> - encoder->bridge = bridge;
>   hdmi->connector.polled = DRM_CONNECTOR_POLL_HPD;
>  
>   drm_connector_helper_add(&hdmi->connector,
> diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
> index 0ee052b7c21a..850bd6509ef1 100644
> --- a/drivers/gpu/drm/drm_bridge.c
> +++ b/drivers/gpu/drm/drm_bridge.c
> @@ -26,6 +26,7 @@
>  #include 
>  
>  #include 
> +#include 
>  
>  /**
>   * DOC: overview
> @@ -92,32 +93,53 @@ void drm_bridge_remove(struct drm_bridge *bridge)
>  EXPORT_SYMBOL(drm_bridge_remove);
>  
>  /**
> - * drm_bridge_attach - associate given bridge to our DRM device
> + * drm_bridge_attach - attach the bridge to an encoder's chain
>   *
> - * @dev: DRM device
> - * @bridge: bridge control structure
> + * @encoder: DRM encoder
> + * @bridge: bridge to attach
>

Re: [PATCH] pinctrl: sh-pfc: r8a7791: Add ADI pinconf support

2016-11-29 Thread Laurent Pinchart
Hi Jacopo,

Thank you for the patch.

On Tuesday 29 Nov 2016 10:11:56 Jacopo Mondi wrote:
> Add pin configuration support for Gyro-ADC, named ADI on r8a7791 SoC.
> 
> The Gyro-ADC supports three different configurations:
> a single ADC (adi and adi_b groups), 2 ADCs selectable through a single
> channel select signal (adi_chsel1 and adi_chsel1_b groups),

I've only briefly looked up at the datasheet, but is that a supported mode ? 
It seems that mode 1 uses 4 channels and mode 2 and 3 use 8 channels.

> up to 4 ADCs
> through 2 channel select signals (adi_chsel2 and adi_chsel2_b groups)
> and up to 8 ADCs through 3 channel select signals (adi_chsel3 and
> adi_chsel3_b groups)
> 
> Signed-off-by: Jacopo Mondi 
> ---
> 
> Compiled only, not tested with an actual ADC.
> 
> For reference only, these are the changes introduced by Geert's private
> review of first sketch of this patch:
> * separate ADI chsel in 3 overlapping groups:
>   the user can now select how many pins to assign to channel selection
>   function.
> 
> ---
>  drivers/pinctrl/sh-pfc/pfc-r8a7791.c | 86 +
>  1 file changed, 86 insertions(+)
> 
> diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a7791.c
> b/drivers/pinctrl/sh-pfc/pfc-r8a7791.c index 7ca37c3..3898747 100644
> --- a/drivers/pinctrl/sh-pfc/pfc-r8a7791.c
> +++ b/drivers/pinctrl/sh-pfc/pfc-r8a7791.c
> @@ -1691,6 +1691,72 @@ static const struct sh_pfc_pin pinmux_pins[] = {
>   PINMUX_GPIO_GP_ALL(),
>  };
> 
> +/* - ADI --- */
> +static const unsigned int adi_pins[] = {
> + /* ADIDATA, ADICS, CLK*/

Missing space before */. Same comment in other places below.

> + RCAR_GP_PIN(6, 24), RCAR_GP_PIN(6, 25), RCAR_GP_PIN(6, 26),
> +};
> +static const unsigned int adi_mux[] = {
> + /* ADIDATA, ADICS, CLK*/
> + ADIDATA_MARK, ADICS_SAMP_MARK, ADICLK_MARK,
> +};
> +static const unsigned int adi_chsel1_pins[] = {
> + /* CHS 0 */
> + RCAR_GP_PIN(6, 27),
> +};
> +static const unsigned int adi_chsel1_mux[] = {
> + /* CHS 0 */
> + ADICHS0_MARK, ADICHS1_MARK, ADICHS2_MARK,

One pin, three mux ?

> +};
> +static const unsigned int adi_chsel2_pins[] = {
> + /* CHS 0, 1 */
> + RCAR_GP_PIN(6, 27), RCAR_GP_PIN(6, 28),
> +};
> +static const unsigned int adi_chsel2_mux[] = {
> + /* CHS 0, 1 */
> + ADICHS0_MARK, ADICHS1_MARK, ADICHS2_MARK,

Two pins, three mux ?

> +};
> +static const unsigned int adi_chsel3_pins[] = {
> + /* CHS 0, 1, 2 */
> + RCAR_GP_PIN(6, 27), RCAR_GP_PIN(6, 28), RCAR_GP_PIN(6, 29),
> +};
> +static const unsigned int adi_chsel3_mux[] = {
> + /* CHS 0, 1, 2 */
> + ADICHS0_MARK, ADICHS1_MARK, ADICHS2_MARK,
> +};
> +static const unsigned int adi_b_pins[] = {
> + /* ADIDATA B, ADICS B, CLK B */
> + RCAR_GP_PIN(5, 25), RCAR_GP_PIN(5, 26), RCAR_GP_PIN(5, 27),
> +};
> +static const unsigned int adi_b_mux[] = {
> + /* ADIDATA B, ADICS B, CLK B */
> + ADIDATA_B_MARK, ADICS_SAMP_B_MARK, ADICLK_B_MARK,
> +};
> +static const unsigned int adi_chsel1_b_pins[] = {
> + /* CHS B 0 */
> + RCAR_GP_PIN(5, 28),
> +};
> +static const unsigned int adi_chsel1_b_mux[] = {
> + /* CHS B 0 */
> + ADICHS0_B_MARK,
> +};
> +static const unsigned int adi_chsel2_b_pins[] = {
> + /* CHS B 0, 1 */
> + RCAR_GP_PIN(5, 28), RCAR_GP_PIN(5, 29),
> +};
> +static const unsigned int adi_chsel2_b_mux[] = {
> + /* CHS B 0, 1 */
> + ADICHS0_B_MARK, ADICHS1_B_MARK,
> +};
> +static const unsigned int adi_chsel3_b_pins[] = {
> + /* CHS B 0, 1, 2 */
> + RCAR_GP_PIN(5, 28), RCAR_GP_PIN(5, 29), RCAR_GP_PIN(5, 30),
> +};
> +static const unsigned int adi_chsel3_b_mux[] = {
> + /* CHS B 0, 1, 2 */
> + ADICHS0_B_MARK, ADICHS1_B_MARK, ADICHS2_B_MARK,
> +};
> +
>  /* - Audio Clock --- */
>  static const unsigned int audio_clk_a_pins[] = {
>   /* CLK */
> @@ -4343,6 +4409,14 @@ static const unsigned int vin2_clk_mux[] = {
>  };
> 
>  static const struct sh_pfc_pin_group pinmux_groups[] = {
> + SH_PFC_PIN_GROUP(adi),
> + SH_PFC_PIN_GROUP(adi_chsel1),
> + SH_PFC_PIN_GROUP(adi_chsel2),
> + SH_PFC_PIN_GROUP(adi_chsel3),
> + SH_PFC_PIN_GROUP(adi_b),
> + SH_PFC_PIN_GROUP(adi_chsel1_b),
> + SH_PFC_PIN_GROUP(adi_chsel2_b),
> + SH_PFC_PIN_GROUP(adi_chsel3_b),
>   SH_PFC_PIN_GROUP(audio_clk_a),
>   SH_PFC_PIN_GROUP(audio_clk_b),
>   SH_PFC_PIN_GROUP(audio_clk_b_b),
> @@ -4687,6 +4761,17 @@ static const struct sh_pfc_pin_group pinmux_groups[]
> = { SH_PFC_PIN_GROUP(vin2_clk),
>  };
> 
> +static const char * const adi_groups[] = {
> + "adi",
> + "adi_chsel1",
> + "adi_chsel2",
> + "adi_chsel3",
> + "adi_b",
> + "adi_chsel1_b",
> + "adi_chsel2_b",
> + "adi_chsel3_b",
> +};
> +
>  static const char * const audio_clk_groups[] = {
>   "audio_clk_a",
>   "audio_clk_b",
> @@ -5192,6 +5277,7 @@ static co

Re: [PATCH v3 04/13] drm: bridge: Detach bridge from encoder at encoder cleanup time

2016-11-29 Thread Laurent Pinchart
Hi Daniel,

On Tuesday 29 Nov 2016 10:48:21 Daniel Vetter wrote:
> On Tue, Nov 29, 2016 at 11:04:34AM +0200, Laurent Pinchart wrote:
> > Most drivers that use bridges forgot to detach them at cleanup time.
> > Instead of fixing them one by one, detach the bridge in the core
> > drm_encoder_cleanup() function.
> > 
> > Signed-off-by: Laurent Pinchart
> > 
> > ---
> > 
> >  drivers/gpu/drm/drm_encoder.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/drm_encoder.c b/drivers/gpu/drm/drm_encoder.c
> > index 5c067719164d..9c1f99646e0d 100644
> > --- a/drivers/gpu/drm/drm_encoder.c
> > +++ b/drivers/gpu/drm/drm_encoder.c
> > @@ -164,6 +164,9 @@ void drm_encoder_cleanup(struct drm_encoder *encoder)
> > 
> >  * the indices on the drm_encoder after us in the encoder_list.
> >  */
> > 
> > +   if (encoder->bridge)
> > +   drm_bridge_detach(encoder->bridge);
> 
> Means we bake in drm_bridge much more as a core thing, but I guess that's
> ok.
> 
> But there's 3 callers of drm_bridge_detach outside of the drm core, can't
> we remove them and drop the EXPORT_SYMBOL for drm_bridge_detach? What's
> it still needed for?

Agreed, I'll fix that.

> I think that cleanup should done in this patch here - drm_bridge_detach
> WARN_ONs if the bridge is already detached ...
>
> > +
> > drm_modeset_lock_all(dev);
> > drm_mode_object_unregister(dev, &encoder->base);
> > kfree(encoder->name);

-- 
Regards,

Laurent Pinchart



Re: [PATCH v3 04/13] drm: bridge: Detach bridge from encoder at encoder cleanup time

2016-11-29 Thread Laurent Pinchart
Hi Archit,

On Tuesday 29 Nov 2016 16:04:08 Archit Taneja wrote:
> On 11/29/2016 02:34 PM, Laurent Pinchart wrote:
> > Most drivers that use bridges forgot to detach them at cleanup time.
> > Instead of fixing them one by one, detach the bridge in the core
> > drm_encoder_cleanup() function.
> > 
> > Signed-off-by: Laurent Pinchart
> > 
> > ---
> > 
> >  drivers/gpu/drm/drm_encoder.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/drm_encoder.c b/drivers/gpu/drm/drm_encoder.c
> > index 5c067719164d..9c1f99646e0d 100644
> > --- a/drivers/gpu/drm/drm_encoder.c
> > +++ b/drivers/gpu/drm/drm_encoder.c
> > @@ -164,6 +164,9 @@ void drm_encoder_cleanup(struct drm_encoder *encoder)
> >  * the indices on the drm_encoder after us in the encoder_list.
> >  */
> > +   if (encoder->bridge)
> > +   drm_bridge_detach(encoder->bridge);
> 
> This would require the kms driver to still detach the remaining
> n - 1 bridges in a possible chain. We could probably detach all of
> them here, or maybe leave detaching of all to the kms driver, and just
> report a warning here.

I'd prefer detaching them all here, but that's a bit intrusive and should be 
tested correctly. The patch series is already growing big, could we do that in 
a separate patch ?

> > +
> > 
> > drm_modeset_lock_all(dev);
> > drm_mode_object_unregister(dev, &encoder->base);
> > kfree(encoder->name);

-- 
Regards,

Laurent Pinchart



Re: [PATCH v3 2/4] thermal: rcar_gen3_thermal: Add R-Car Gen3 thermal driver

2016-11-29 Thread Wolfram Sang
Hi Geert,

thanks for the prompt review!

> > +/* Structure for thermal temperature calculation */
> > +struct equation_coefs {
> > +   long a1;
> > +   long b1;
> > +   long a2;
> > +   long b2;
> 
> Why long? Long has a different size for 32-bit and 64-bit platforms.
> I know this is a driver for arm64, but if you need 64-bits, you want to make
> this clear using s64, or long long.

I'll check if int will do.

> > +static inline void rcar_gen3_thermal_write(struct rcar_gen3_thermal_tsc 
> > *tsc,
> > +u32 reg, u32 data)
> > +{
> > +   iowrite32(data, tsc->base + reg);
> > +}
> 
> ... so using the "convenience" wrappers is more (typing) work than using
> io{read,write}32() directly?

TBH I don't favor such macros, but since they are in quite some Renesas
drivers, I kept them in the first take for consistency reasons.

> > +   tj_2 = (CODETSD((ptat[1] - ptat[2]) * 137)
> > +   / (ptat[0] - ptat[2])) - CODETSD(41);
> 
> Isn't "* 1000" easier to read then CODETSD()?

Probably. I'd think there can be done even more to make this code a tad
more readable. For the first take, I'd like to keep these formulas,
though. They come from the BSP and are the only documentation about how
to calculate the temperature which we currently have. Changing them
will obsolete all the testing done so far.


> > +   /* calculate coefficients for linear equation */
> > +   a1_num = CODETSD(thcode[1] - thcode[2]);
> > +   a1_den = tj_2 - TJ_3;
> > +   a1 = (1 * a1_num) / a1_den;
> > +   b1 = (1 * thcode[2]) - ((a1 * TJ_3) / 1000);
> 
> Rounding needed for / 1000?

Ditto.

> > +static int rcar_gen3_thermal_probe(struct platform_device *pdev)
> > +{
> > +   struct rcar_gen3_thermal_priv *priv;
> > +   struct device *dev = &pdev->dev;
> > +   struct resource *res;
> > +   struct thermal_zone_device *zone;
> > +   int ret, i;
> 
> unsigned int i;

I'll likely produce more bugs if 'i' is not an int ;)

> 
> > +   const struct rcar_gen3_thermal_data *match_data = 
> > of_device_get_match_data(dev);
> > +
> > +   /* default values if FUSEs are missing */
> > +   int ptat[3] = { 2351, 1509, 435 };
> 
> unsigned?
> 
> > +   int thcode[TSC_MAX_NUM][3] = {
> 
> unsigned?

Had that before, didn't work. Since the calculation involves
substraction with other ints, I prefer to have them all the same type as
the fix.

> > +   tsc = devm_kzalloc(dev, sizeof(*tsc), GFP_KERNEL);
> > +   if (!tsc)
> > +   return -ENOMEM;
> 
> Missing pm_runtime_put() etc.?
> 
> ret = -ENOMEM;
> goto error_unregister;

Yes!

Regards,

   Wolfram



signature.asc
Description: PGP signature


Re: [PATCH v3 03/13] drm: bridge: Link encoder and bridge in core code

2016-11-29 Thread Laurent Pinchart
Hi Daniel,

On Tuesday 29 Nov 2016 20:02:01 Laurent Pinchart wrote:
> On Tuesday 29 Nov 2016 11:05:27 Daniel Vetter wrote:
> > On Tue, Nov 29, 2016 at 11:43:19AM +0200, Laurent Pinchart wrote:
> >> On Tuesday 29 Nov 2016 10:35:24 Daniel Vetter wrote:
> >>> On Tue, Nov 29, 2016 at 11:04:33AM +0200, Laurent Pinchart wrote:
>  Instead of linking encoders and bridges in every driver (and getting
>  it wrong half of the time, as many drivers forget to set the
>  drm_bridge encoder pointer), do so in core code. The
>  drm_bridge_attach() function needs the encoder and optional previous
>  bridge to perform that task, update all the callers.
>  
>  Signed-off-by: Laurent Pinchart
>  
>  ---

[snip]

>  diff --git a/drivers/gpu/drm/drm_bridge.c
>  b/drivers/gpu/drm/drm_bridge.c
>  index 0ee052b7c21a..850bd6509ef1 100644
>  --- a/drivers/gpu/drm/drm_bridge.c
>  +++ b/drivers/gpu/drm/drm_bridge.c
> 
> [snip]
> 
>  -int drm_bridge_attach(struct drm_device *dev, struct drm_bridge
>  *bridge)
>  +int drm_bridge_attach(struct drm_encoder *encoder, struct drm_bridge
>  *bridge,
>  +  struct drm_bridge *previous)
>   {
>  -if (!dev || !bridge)
>  +int ret;
>  +
>  +if (!encoder || !bridge)
>  +return -EINVAL;
>  +
>  +if (previous && (!previous->dev || previous->encoder !=
>  encoder))
>   return -EINVAL;
> >>> 
> >>> Not sure we want to allow setting both encoder and bridge for chaining.
> >>> I'd kinda expect that for chained use-case the bridge doesn't care that
> >>> much who started the chain. And if not we can always create a helper to
> >>> look up the drm_encoder.
> >> 
> >> As bridge drivers currently create connectors (I'd like to change that
> >> at some point, but one thing at a time) they need to call
> >> drm_mode_connector_attach_encoder() and thus need to have access to the
> >> drm_encoder object at the beginning of the chain. The drm_bridge
> >> structure has an encoder field, it seems to me that the easiest is to
> >> always populate it, regardless of the position of the bridge in the
> >> chain.
> > 
> > I mean the function inteface, not what we fill out in the drm_bridge
> > struct. I.e. instead of expecting callers to give you the encoder for
> > 
> > chained bridges, look it up yourself:
> > bridge->encoder = previous : previous->encoder ? encoder;
> > 
> > Or something  like that. Just feels confusing to connect a bridge to both
> > a bridge _and_ the first encoder.
> 
> Right. Archit proposed doing it the other way around:
> 
> previous = encoder->bridge;
> while (previous && previous->next)
> previous = previous->next;
> 
> That would simplify the API, at the cost of preventing us from catching
> drivers that attach bridges in the wrong order (through the !previous->dev
> check that you suggested should be turned into a WARN_ON). The previous-
> 
> >encoder != encoder check is also a safety net.
> 
> Any opinion on which option you like better ? I'd be very tempted to go for
> Archit's proposal as it removes the previous parameter from the API, if it
> wasn't for the loss of sanity checking.
> 
> > Wrt creating the connector: Some helpers to make that easier could be
> > useful, and probably we need a separate ->register callback. That's needed
> > for proper demidlayered init/teardown sequence anyway, and then the
> > drm_bridge.c code make sure to only call ->register for the very last
> > bridge. Other bridges could still create ther connectors (less conditions
> > in the code), as long as they don't register them no one will ever see
> > them ;-)

One thing at a time, we'll get there :-)

-- 
Regards,

Laurent Pinchart



Re: [PATCH v2 01/13] devicetree/bindings: display: Document common panel properties

2016-11-29 Thread Laurent Pinchart
Hi Rob,

On Tuesday 29 Nov 2016 09:14:09 Rob Herring wrote:
> On Tue, Nov 29, 2016 at 2:27 AM, Laurent Pinchart wrote:
> > On Tuesday 22 Nov 2016 11:36:55 Laurent Pinchart wrote:
> >> On Monday 21 Nov 2016 10:48:15 Rob Herring wrote:
> >>> On Sat, Nov 19, 2016 at 05:28:01AM +0200, Laurent Pinchart wrote:
>  Document properties common to several display panels in a central
>  location that can be referenced by the panel device tree bindings.
> >>> 
> >>> Looks good. Just one comment...
> >>> 
> >>> [...]
> >>> 
>  +Connectivity
>  +
>  +
>  +- ports: Panels receive video data through one or multiple
>  connections. While
>  +  the nature of those connections is specific to the panel type, the
>  +  connectivity is expressed in a standard fashion using ports as
>  specified in
>  +  the device graph bindings defined in
>  +  Documentation/devicetree/bindings/graph.txt.
> >>> 
> >>> We allow panels to either use graph binding or be a child of the
> >>> display controller.
> >> 
> >> I knew that some display controllers use a phandle to the panel (see the
> >> fsl,panel and nvidia,panel properties), but I didn't know we had panels
> >> as children of display controller nodes. I don't think we should allow
> >> that for anything but DSI panels, as the DT hierarchy is based on control
> >> buses. Are you sure we have other panels instantiated through that
> >> mechanism ?
>
> Some panels have no control bus, so were do we place them?

I'd say under the root node, like all similar control-less devices.

> I would say the hierarchy is based on buses with a preference for the
> control bus when there are multiple buses. I'm not a fan of just sticking
> things are the top level.

OK, so much for my comment a few lines up :-)

The problem with placing non-DSI panels as children of the display controller 
and not using OF graph is that the panel bindings become dependent of the 
display controller being used. A display controller using OF graph would 
require the panel to do the same, while a display controller expecting a panel 
child node (with a specific name) would require DT properties for the panel 
node.

I'm also not sure the complexity of OF graph is really that prohibitive if you 
compare it to panels as child nodes. To get the panel driver to bind to the 
panel DT node the display controller driver would need to create a platform 
device for the panel and register it. That's not very difficult, but parsing a 
single port and endpoint isn't either (and we could even provide a helper 
function for that, a version of of_drm_find_panel() that would take as an 
argument the display controller device node instead of the panel device node).

> > Ping ?
> > 
> > Please note that this file documents properties common to multiple panel
> > DT bindings, but in no way makes it mandatory to use the OF graph bindings
> > for panels. The decision is left to individual bindings.
> 
> It is mandatory in the sense that we don't want more cases of "fsl,panel".

That I agree with :-)

-- 
Regards,

Laurent Pinchart



Re: [PATCH v3 03/13] drm: bridge: Link encoder and bridge in core code

2016-11-29 Thread Laurent Pinchart
Hi Daniel,

On Tuesday 29 Nov 2016 11:05:27 Daniel Vetter wrote:
> On Tue, Nov 29, 2016 at 11:43:19AM +0200, Laurent Pinchart wrote:
> > On Tuesday 29 Nov 2016 10:35:24 Daniel Vetter wrote:
> >> On Tue, Nov 29, 2016 at 11:04:33AM +0200, Laurent Pinchart wrote:
> >>> Instead of linking encoders and bridges in every driver (and getting
> >>> it wrong half of the time, as many drivers forget to set the
> >>> drm_bridge encoder pointer), do so in core code. The
> >>> drm_bridge_attach() function needs the encoder and optional previous
> >>> bridge to perform that task, update all the callers.
> >>> 
> >>> Signed-off-by: Laurent Pinchart
> >>> 
> >>> ---
> >>> 
> >>>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c   |  4 +-
> >>>  drivers/gpu/drm/bridge/analogix/analogix_dp_core.c |  4 +-
> >>>  drivers/gpu/drm/bridge/dw-hdmi.c   |  3 +-
> >>>  drivers/gpu/drm/drm_bridge.c   | 46 +
> >>>  drivers/gpu/drm/drm_simple_kms_helper.c|  4 +-
> >>>  drivers/gpu/drm/exynos/exynos_dp.c |  5 +--
> >>>  drivers/gpu/drm/exynos/exynos_drm_dsi.c|  6 +--
> >>>  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c  |  5 +--
> >>>  drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c   |  5 +--
> >>>  drivers/gpu/drm/imx/imx-ldb.c  |  6 +--
> >>>  drivers/gpu/drm/imx/parallel-display.c |  4 +-
> >>>  drivers/gpu/drm/mediatek/mtk_dpi.c |  8 ++--
> >>>  drivers/gpu/drm/mediatek/mtk_dsi.c | 24 ++-
> >>>  drivers/gpu/drm/mediatek/mtk_hdmi.c| 11 +++---
> >>>  drivers/gpu/drm/msm/dsi/dsi_manager.c  | 17 +---
> >>>  drivers/gpu/drm/msm/edp/edp_bridge.c   |  2 +-
> >>>  drivers/gpu/drm/msm/hdmi/hdmi_bridge.c |  2 +-
> >>>  drivers/gpu/drm/rcar-du/rcar_du_hdmienc.c  |  5 +--
> >>>  drivers/gpu/drm/sti/sti_dvo.c  |  3 +-
> >>>  drivers/gpu/drm/sti/sti_hda.c  |  3 +-
> >>>  drivers/gpu/drm/sti/sti_hdmi.c |  3 +-
> >>>  drivers/gpu/drm/sun4i/sun4i_rgb.c  | 13 +++---
> >>>  include/drm/drm_bridge.h   |  3 +-
> >>>  23 files changed, 83 insertions(+), 103 deletions(-)
> > 
> > [snip]
> > 
> >>> diff --git a/drivers/gpu/drm/drm_bridge.c
> >>> b/drivers/gpu/drm/drm_bridge.c
> >>> index 0ee052b7c21a..850bd6509ef1 100644
> >>> --- a/drivers/gpu/drm/drm_bridge.c
> >>> +++ b/drivers/gpu/drm/drm_bridge.c

[snip]

> >>> -int drm_bridge_attach(struct drm_device *dev, struct drm_bridge
> >>> *bridge)
> >>> +int drm_bridge_attach(struct drm_encoder *encoder, struct drm_bridge
> >>> *bridge,
> >>> +   struct drm_bridge *previous)
> >>>  {
> >>> - if (!dev || !bridge)
> >>> + int ret;
> >>> +
> >>> + if (!encoder || !bridge)
> >>> + return -EINVAL;
> >>> +
> >>> + if (previous && (!previous->dev || previous->encoder != encoder))
> >>>   return -EINVAL;
> >> 
> >> Not sure we want to allow setting both encoder and bridge for chaining.
> >> I'd kinda expect that for chained use-case the bridge doesn't care that
> >> much who started the chain. And if not we can always create a helper to
> >> look up the drm_encoder.
> > 
> > As bridge drivers currently create connectors (I'd like to change that at
> > some point, but one thing at a time) they need to call
> > drm_mode_connector_attach_encoder() and thus need to have access to the
> > drm_encoder object at the beginning of the chain. The drm_bridge structure
> > has an encoder field, it seems to me that the easiest is to always
> > populate it, regardless of the position of the bridge in the chain.
> 
> I mean the function inteface, not what we fill out in the drm_bridge
> struct. I.e. instead of expecting callers to give you the encoder for
> chained bridges, look it up yourself:
> 
>   bridge->encoder = previous : previous->encoder ? encoder;
> 
> Or something  like that. Just feels confusing to connect a bridge to both
> a bridge _and_ the first encoder.

Right. Archit proposed doing it the other way around:

previous = encoder->bridge;
while (previous && previous->next)
previous = previous->next;

That would simplify the API, at the cost of preventing us from catching 
drivers that attach bridges in the wrong order (through the !previous->dev 
check that you suggested should be turned into a WARN_ON). The previous-
>encoder != encoder check is also a safety net.

Any opinion on which option you like better ? I'd be very tempted to go for 
Archit's proposal as it removes the previous parameter from the API, if it 
wasn't for the loss of sanity checking.

> Wrt creating the connector: Some helpers to make that easier could be
> useful, and probably we need a separate ->register callback. That's needed
> for proper demidlayered init/teardown sequence anyway, and then the
> drm_bridge.c code make sure to only call ->register for the

Re: [PATCH v3 03/13] drm: bridge: Link encoder and bridge in core code

2016-11-29 Thread Laurent Pinchart
Hi Archit,

On Tuesday 29 Nov 2016 15:57:06 Archit Taneja wrote:
> On 11/29/2016 02:34 PM, Laurent Pinchart wrote:
> > Instead of linking encoders and bridges in every driver (and getting it
> > wrong half of the time, as many drivers forget to set the drm_bridge
> > encoder pointer), do so in core code. The drm_bridge_attach() function
> > needs the encoder and optional previous bridge to perform that task,
> > update all the callers.
> > 
> > Signed-off-by: Laurent Pinchart
> > 
> > ---
> > 
> >  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c   |  4 +-
> >  drivers/gpu/drm/bridge/analogix/analogix_dp_core.c |  4 +-
> >  drivers/gpu/drm/bridge/dw-hdmi.c   |  3 +-
> >  drivers/gpu/drm/drm_bridge.c   | 46 -
> >  drivers/gpu/drm/drm_simple_kms_helper.c|  4 +-
> >  drivers/gpu/drm/exynos/exynos_dp.c |  5 +--
> >  drivers/gpu/drm/exynos/exynos_drm_dsi.c|  6 +--
> >  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c  |  5 +--
> >  drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c   |  5 +--
> >  drivers/gpu/drm/imx/imx-ldb.c  |  6 +--
> >  drivers/gpu/drm/imx/parallel-display.c |  4 +-
> >  drivers/gpu/drm/mediatek/mtk_dpi.c |  8 ++--
> >  drivers/gpu/drm/mediatek/mtk_dsi.c | 24 ++-
> >  drivers/gpu/drm/mediatek/mtk_hdmi.c| 11 +++---
> >  drivers/gpu/drm/msm/dsi/dsi_manager.c  | 17 +---
> >  drivers/gpu/drm/msm/edp/edp_bridge.c   |  2 +-
> >  drivers/gpu/drm/msm/hdmi/hdmi_bridge.c |  2 +-
> >  drivers/gpu/drm/rcar-du/rcar_du_hdmienc.c  |  5 +--
> >  drivers/gpu/drm/sti/sti_dvo.c  |  3 +-
> >  drivers/gpu/drm/sti/sti_hda.c  |  3 +-
> >  drivers/gpu/drm/sti/sti_hdmi.c |  3 +-
> >  drivers/gpu/drm/sun4i/sun4i_rgb.c  | 13 +++---
> >  include/drm/drm_bridge.h   |  3 +-
> >  23 files changed, 83 insertions(+), 103 deletions(-)

[snip]

> > diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
> > index 0ee052b7c21a..850bd6509ef1 100644
> > --- a/drivers/gpu/drm/drm_bridge.c
> > +++ b/drivers/gpu/drm/drm_bridge.c

[snip]

> > @@ -92,32 +93,53 @@ void drm_bridge_remove(struct drm_bridge *bridge)
> >  EXPORT_SYMBOL(drm_bridge_remove);
> >  
> >  /**
> > - * drm_bridge_attach - associate given bridge to our DRM device
> > + * drm_bridge_attach - attach the bridge to an encoder's chain
> >   *
> > - * @dev: DRM device
> > - * @bridge: bridge control structure
> > + * @encoder: DRM encoder
> > + * @bridge: bridge to attach
> > + * @previous: previous bridge in the chain (optional)
> >   *
> > - * Called by a kms driver to link one of our encoder/bridge to the given
> > - * bridge.
> > + * Called by a kms driver to link the bridge to an encoder's chain. The
> > previous
> > + * argument specifies the previous bridge in the chain. If NULL, the
> > bridge is
> > + * linked directly at the encoder's output. Otherwise it is linked at the
> > + * previous bridge's output.
> >   *
> > - * Note that setting up links between the bridge and our encoder/bridge
> > - * objects needs to be handled by the kms driver itself.
> > + * If non-NULL the previous bridge must be already attached by a call to
> > this
> > + * function.
> >   *
> >   * RETURNS:
> >   * Zero on success, error code on failure
> >   */
> > -int drm_bridge_attach(struct drm_device *dev, struct drm_bridge *bridge)
> > +int drm_bridge_attach(struct drm_encoder *encoder, struct drm_bridge
> > *bridge,
> > + struct drm_bridge *previous)
> >  {
> > -   if (!dev || !bridge)
> > +   int ret;
> > +
> > +   if (!encoder || !bridge)
> > +   return -EINVAL;
> 
> I think we could derive previous from the encoder itself. Something like:
> 
>   previous = encoder->bridge;
>   while (previous && previous->next)
>   previous = previous->next;

That's a very good point. It would however prevent us from catching drivers 
that attach bridges in the wrong order, which the !previous->dev currently 
allows us to do (and it should be turned into a WARN_ON as Daniel proposed).

I'm fine losing that ability, as your proposal makes the API simpler. I'll let 
you decide, which option do you prefer ?

> > +
> > +   if (previous && (!previous->dev || previous->encoder != encoder))
> > return -EINVAL;
> > 
> > if (bridge->dev)
> > return -EBUSY;
> > 
> > -   bridge->dev = dev;
> > +   bridge->dev = encoder->dev;
> > +   bridge->encoder = encoder;
> > +
> > +   if (bridge->funcs->attach) {
> > +   ret = bridge->funcs->attach(bridge);
> > +   if (ret < 0) {
> > +   bridge->dev = NULL;
> > +   bridge->encoder = NULL;
> > +   return ret;
> > +   }
> > +   }
> > 
> > -   if (bridge->funcs->attach)
> > -   r

Re: [PATCH v3 09/13] drm: Add encoder_type field to the drm_bridge structure

2016-11-29 Thread Laurent Pinchart
Hi Daniel,

On Tuesday 29 Nov 2016 11:27:20 Daniel Vetter wrote:
> On Tue, Nov 29, 2016 at 11:58:44AM +0200, Laurent Pinchart wrote:
> > On Tuesday 29 Nov 2016 10:56:53 Daniel Vetter wrote:
> >> On Tue, Nov 29, 2016 at 11:04:39AM +0200, Laurent Pinchart wrote:
> >>> The drm_bridge object models on- or off-chip hardware encoders and
> >>> provide an abstract control API to display drivers. In order to help
> >>> display drivers creating the right kind of drm_encoder object, expose
> >>> the type of the hardware encoder associated with each bridge.
> >>> 
> >>> Signed-off-by: Laurent Pinchart
> >>> 
> >> 
> >> DRM_MODE_ENCODER_BRIDGE. Problem solved, because in reality no one cares
> >> one iota about the encoder type.
> > 
> > It's exposed to userspace though, are you 100% sure we won't break
> > anything ?
>
> We've added DP, DSI, DPMST and DPI encoder types thus far, no one
> screamed.

In that case why don't we go one step further and remove the encoder type 
completely ? We can't remove the field from the API, but we can hardcode it to 
a single value.

There are however drivers that rely on the encoder type (radeon, nouveau, sti, 
amdgpu, msm and rcar-du, but I'll fix the last one) so we'd need to address 
that first. If we don't want to remove the encoder_type field from in-kernel 
structures and let drivers use it, then I don't think DRM_MODE_ENCODER_BRIDGE 
would be a good option, we should report the real type instead.

> We should be fine I think. Connector types are a bit different, userspace
> (at least X) wants to pretty-print them for xrandr names.

-- 
Regards,

Laurent Pinchart



Re: [PATCH/RFC linux-mtd] mtd: sh_flctl: Remove sh7372 and device tree support

2016-11-29 Thread Rich Felker
On Tue, Nov 29, 2016 at 06:23:38PM +0100, Geert Uytterhoeven wrote:
> Hi Rich,
> 
> On Tue, Nov 29, 2016 at 6:09 PM, Rich Felker  wrote:
> > On Fri, Nov 25, 2016 at 08:33:01AM +0100, Simon Horman wrote:
> >> Commit edf4100906044225 ("ARM: shmobile: sh7372 dtsi: Remove Legacy file")
> >> removed the sh7272 SoC from the kernel in v4.1.
> >>
> >> This patch removes support for the sh7272 SoC from the sh_flctl driver.
> >> As that SoC was the only user of device tree support also remove that
> >> from the driver.
> >>
> >> In essence it reverts commit 7c8f680e96ed ("mtd: sh_flctl: Add device tree
> >> support"). This commit may be used as a reference for re-adding device
> >> tree support to this driver if a need for it is found in future.
> >>
> >> This commit has been build-testesd against the ap325rxa_defconfig.
> >> I do not have access to the hardware to perform run-time testing
> >> on that board which appears to be the only remaining user of this driver.
> >> Signed-off-by: Simon Horman 
> >> ---
> >>  .../devicetree/bindings/mtd/flctl-nand.txt | 49 ---
> >>  drivers/mtd/nand/sh_flctl.c| 70 
> >> +++---
> >>  2 files changed, 8 insertions(+), 111 deletions(-)
> >>  delete mode 100644 Documentation/devicetree/bindings/mtd/flctl-nand.txt
> >
> > While I'm not the maintainer for this (and I'm not clear that the
> > linux-sh list even should have been cc'd; it seems to be shmobile arm
> > soc stuff rather than sh arch) I think this is a bad change. If the
> > driver is completely unused, it should just be removed, but if there
> 
> Its sole remaining in-kernel user is arch/sh/boards/mach-ap325rxa/setup.c.

If indeed there's a user in arch/sh, it makes even more sense not to
remove it. arch/sh/boards/* is intended to be removed with only DT
support remaining, once drivers have DT support. :-)

> > are remaining users that are still using legacy platform device
> > bindings, they should gradually be transitioned to device tree, and
> > having the device tree support still present makes it easier to do
> > that.
> 
> Enjoy converting ap325rxa to DT ;-)

Not sure if that's expected to be worse than the rest of it and if it
makes sense to convert or just remove. But I don't think changes that
put us farther away from DT-only make sense.

> > Removing the *bindings* is even worse, as these are a permanent
> > interface between hardware/firmware and software.
> 
> While it is a permanent interface, it is only a git revert away...

Indeed, but one that requires coordination between subsystem
maintainers to get it back in a working state.

Rich


Re: [PATCH/RFC linux-mtd] mtd: sh_flctl: Remove sh7372 and device tree support

2016-11-29 Thread Geert Uytterhoeven
Hi Rich,

On Tue, Nov 29, 2016 at 6:09 PM, Rich Felker  wrote:
> On Fri, Nov 25, 2016 at 08:33:01AM +0100, Simon Horman wrote:
>> Commit edf4100906044225 ("ARM: shmobile: sh7372 dtsi: Remove Legacy file")
>> removed the sh7272 SoC from the kernel in v4.1.
>>
>> This patch removes support for the sh7272 SoC from the sh_flctl driver.
>> As that SoC was the only user of device tree support also remove that
>> from the driver.
>>
>> In essence it reverts commit 7c8f680e96ed ("mtd: sh_flctl: Add device tree
>> support"). This commit may be used as a reference for re-adding device
>> tree support to this driver if a need for it is found in future.
>>
>> This commit has been build-testesd against the ap325rxa_defconfig.
>> I do not have access to the hardware to perform run-time testing
>> on that board which appears to be the only remaining user of this driver.
>> Signed-off-by: Simon Horman 
>> ---
>>  .../devicetree/bindings/mtd/flctl-nand.txt | 49 ---
>>  drivers/mtd/nand/sh_flctl.c| 70 
>> +++---
>>  2 files changed, 8 insertions(+), 111 deletions(-)
>>  delete mode 100644 Documentation/devicetree/bindings/mtd/flctl-nand.txt
>
> While I'm not the maintainer for this (and I'm not clear that the
> linux-sh list even should have been cc'd; it seems to be shmobile arm
> soc stuff rather than sh arch) I think this is a bad change. If the
> driver is completely unused, it should just be removed, but if there

Its sole remaining in-kernel user is arch/sh/boards/mach-ap325rxa/setup.c.

> are remaining users that are still using legacy platform device
> bindings, they should gradually be transitioned to device tree, and
> having the device tree support still present makes it easier to do
> that.

Enjoy converting ap325rxa to DT ;-)

> Removing the *bindings* is even worse, as these are a permanent
> interface between hardware/firmware and software.

While it is a permanent interface, it is only a git revert away...

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds


Re: [PATCH/RFC linux-mtd] mtd: sh_flctl: Remove sh7372 and device tree support

2016-11-29 Thread Rich Felker
On Fri, Nov 25, 2016 at 08:33:01AM +0100, Simon Horman wrote:
> Commit edf4100906044225 ("ARM: shmobile: sh7372 dtsi: Remove Legacy file")
> removed the sh7272 SoC from the kernel in v4.1.
> 
> This patch removes support for the sh7272 SoC from the sh_flctl driver.
> As that SoC was the only user of device tree support also remove that
> from the driver.
> 
> In essence it reverts commit 7c8f680e96ed ("mtd: sh_flctl: Add device tree
> support"). This commit may be used as a reference for re-adding device
> tree support to this driver if a need for it is found in future.
> 
> This commit has been build-testesd against the ap325rxa_defconfig.
> I do not have access to the hardware to perform run-time testing
> on that board which appears to be the only remaining user of this driver.
> Signed-off-by: Simon Horman 
> ---
>  .../devicetree/bindings/mtd/flctl-nand.txt | 49 ---
>  drivers/mtd/nand/sh_flctl.c| 70 
> +++---
>  2 files changed, 8 insertions(+), 111 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/mtd/flctl-nand.txt

While I'm not the maintainer for this (and I'm not clear that the
linux-sh list even should have been cc'd; it seems to be shmobile arm
soc stuff rather than sh arch) I think this is a bad change. If the
driver is completely unused, it should just be removed, but if there
are remaining users that are still using legacy platform device
bindings, they should gradually be transitioned to device tree, and
having the device tree support still present makes it easier to do
that.

Removing the *bindings* is even worse, as these are a permanent
interface between hardware/firmware and software.

Rich


Re: [PATCH v3 03/13] drm: bridge: Link encoder and bridge in core code

2016-11-29 Thread Stefan Agner
On 2016-11-29 01:04, Laurent Pinchart wrote:
> Instead of linking encoders and bridges in every driver (and getting it
> wrong half of the time, as many drivers forget to set the drm_bridge
> encoder pointer), do so in core code. The drm_bridge_attach() function
> needs the encoder and optional previous bridge to perform that task,
> update all the callers.
> 
> Signed-off-by: Laurent Pinchart 
> ---
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c   |  4 +-
>  drivers/gpu/drm/bridge/analogix/analogix_dp_core.c |  4 +-
>  drivers/gpu/drm/bridge/dw-hdmi.c   |  3 +-
>  drivers/gpu/drm/drm_bridge.c   | 46 
> --
>  drivers/gpu/drm/drm_simple_kms_helper.c|  4 +-
>  drivers/gpu/drm/exynos/exynos_dp.c |  5 +--
>  drivers/gpu/drm/exynos/exynos_drm_dsi.c|  6 +--
>  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c  |  5 +--

For DCU

Acked-by: Stefan Agner 

>  drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c   |  5 +--
>  drivers/gpu/drm/imx/imx-ldb.c  |  6 +--
>  drivers/gpu/drm/imx/parallel-display.c |  4 +-
>  drivers/gpu/drm/mediatek/mtk_dpi.c |  8 ++--
>  drivers/gpu/drm/mediatek/mtk_dsi.c | 24 ++-
>  drivers/gpu/drm/mediatek/mtk_hdmi.c| 11 +++---
>  drivers/gpu/drm/msm/dsi/dsi_manager.c  | 17 +---
>  drivers/gpu/drm/msm/edp/edp_bridge.c   |  2 +-
>  drivers/gpu/drm/msm/hdmi/hdmi_bridge.c |  2 +-
>  drivers/gpu/drm/rcar-du/rcar_du_hdmienc.c  |  5 +--
>  drivers/gpu/drm/sti/sti_dvo.c  |  3 +-
>  drivers/gpu/drm/sti/sti_hda.c  |  3 +-
>  drivers/gpu/drm/sti/sti_hdmi.c |  3 +-
>  drivers/gpu/drm/sun4i/sun4i_rgb.c  | 13 +++---
>  include/drm/drm_bridge.h   |  3 +-
>  23 files changed, 83 insertions(+), 103 deletions(-)
> 
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c
> b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c
> index 6119b5085501..e7799b6ee829 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c
> @@ -230,9 +230,7 @@ static int atmel_hlcdc_attach_endpoint(struct
> drm_device *dev,
>   of_node_put(np);
>  
>   if (bridge) {
> - output->encoder.bridge = bridge;
> - bridge->encoder = &output->encoder;
> - ret = drm_bridge_attach(dev, bridge);
> + ret = drm_bridge_attach(&output->encoder, bridge, NULL);
>   if (!ret)
>   return 0;
>   }
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> index 6e0447f329a2..1835f1fdad19 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> @@ -1227,12 +1227,10 @@ static int analogix_dp_create_bridge(struct
> drm_device *drm_dev,
>  
>   dp->bridge = bridge;
>  
> - dp->encoder->bridge = bridge;
>   bridge->driver_private = dp;
> - bridge->encoder = dp->encoder;
>   bridge->funcs = &analogix_dp_bridge_funcs;
>  
> - ret = drm_bridge_attach(drm_dev, bridge);
> + ret = drm_bridge_attach(dp->encoder, bridge, NULL);
>   if (ret) {
>   DRM_ERROR("failed to attach drm bridge\n");
>   return -EINVAL;
> diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c 
> b/drivers/gpu/drm/bridge/dw-hdmi.c
> index b71088dab268..432e0e3fff72 100644
> --- a/drivers/gpu/drm/bridge/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/dw-hdmi.c
> @@ -1841,13 +1841,12 @@ static int dw_hdmi_register(struct drm_device
> *drm, struct dw_hdmi *hdmi)
>   hdmi->bridge = bridge;
>   bridge->driver_private = hdmi;
>   bridge->funcs = &dw_hdmi_bridge_funcs;
> - ret = drm_bridge_attach(drm, bridge);
> + ret = drm_bridge_attach(encoder, bridge, NULL);
>   if (ret) {
>   DRM_ERROR("Failed to initialize bridge with drm\n");
>   return -EINVAL;
>   }
>  
> - encoder->bridge = bridge;
>   hdmi->connector.polled = DRM_CONNECTOR_POLL_HPD;
>  
>   drm_connector_helper_add(&hdmi->connector,
> diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
> index 0ee052b7c21a..850bd6509ef1 100644
> --- a/drivers/gpu/drm/drm_bridge.c
> +++ b/drivers/gpu/drm/drm_bridge.c
> @@ -26,6 +26,7 @@
>  #include 
>  
>  #include 
> +#include 
>  
>  /**
>   * DOC: overview
> @@ -92,32 +93,53 @@ void drm_bridge_remove(struct drm_bridge *bridge)
>  EXPORT_SYMBOL(drm_bridge_remove);
>  
>  /**
> - * drm_bridge_attach - associate given bridge to our DRM device
> + * drm_bridge_attach - attach the bridge to an encoder's chain
>   *
> - * @dev: DRM device
> - * @bridge: bridge control structure
> + * @encoder: DRM encoder
> + * @bridge: bridge to attach
> + * @previous: previous bridge 

Re: [PATCH v2 01/13] devicetree/bindings: display: Document common panel properties

2016-11-29 Thread Rob Herring
On Tue, Nov 29, 2016 at 2:27 AM, Laurent Pinchart
 wrote:
> Hi Rob,
>
> On Tuesday 22 Nov 2016 11:36:55 Laurent Pinchart wrote:
>> On Monday 21 Nov 2016 10:48:15 Rob Herring wrote:
>> > On Sat, Nov 19, 2016 at 05:28:01AM +0200, Laurent Pinchart wrote:
>> >> Document properties common to several display panels in a central
>> >> location that can be referenced by the panel device tree bindings.
>> >
>> > Looks good. Just one comment...
>> >
>> > [...]
>> >
>> >> +Connectivity
>> >> +
>> >> +
>> >> +- ports: Panels receive video data through one or multiple connections.
>> >> While
>> >> +  the nature of those connections is specific to the panel type, the
>> >> +  connectivity is expressed in a standard fashion using ports as
>> >> specified in
>> >> +  the device graph bindings defined in
>> >> +  Documentation/devicetree/bindings/graph.txt.
>> >
>> > We allow panels to either use graph binding or be a child of the display
>> > controller.
>>
>> I knew that some display controllers use a phandle to the panel (see the
>> fsl,panel and nvidia,panel properties), but I didn't know we had panels as
>> children of display controller nodes. I don't think we should allow that for
>> anything but DSI panels, as the DT hierarchy is based on control buses. Are
>> you sure we have other panels instantiated through that mechanism ?

Some panels have no control bus, so were do we place them? I would say
the hierarchy is based on buses with a preference for the control bus
when there are multiple buses. I'm not a fan of just sticking things
are the top level.

> Ping ?
>
> Please note that this file documents properties common to multiple panel DT
> bindings, but in no way makes it mandatory to use the OF graph bindings for
> panels. The decision is left to individual bindings.

It is mandatory in the sense that we don't want more cases of "fsl,panel".

Rob


Re: [PATCH v3 0/2] Ajust lockdep static allocations for sparc

2016-11-29 Thread Peter Zijlstra
On Tue, Nov 29, 2016 at 02:39:20PM +0100, Geert Uytterhoeven wrote:

> > Not understanding, why would a user ever need it? The platform knows if
> > its has funny boot image size limits, no?
> 
> The boot loader does not come with the kernel, so the platform cannot
> know for sure.

Why would anybody use a bootloader that imposes weird restrictions on
their platform? That is, isn't that simply a broken bootloader?


Re: [PATCH v3 0/2] Ajust lockdep static allocations for sparc

2016-11-29 Thread Geert Uytterhoeven
Hi Peter,

On Tue, Nov 29, 2016 at 2:31 PM, Peter Zijlstra  wrote:
> On Tue, Nov 29, 2016 at 02:26:47PM +0100, Geert Uytterhoeven wrote:
>> On Tue, Nov 29, 2016 at 1:29 PM, Peter Zijlstra  wrote:
>> > On Tue, Nov 29, 2016 at 12:52:04PM +0100, Geert Uytterhoeven wrote:
>
>> >> Not because of platforms with not limited memory, but because of platforms
>> >> with boot loaders that have silly kernel size limitations, and start
>> >> scribbling over the DTB or even theirselves when copying a large kernel 
>> >> image.
>> >
>> > Right, that's the weird platforms clause above, and those can select the
>> > option.
>>
>> Their Kconfig files can select it. The users can't.
>> What about making it visible depending on EXPERT?
>
> Not understanding, why would a user ever need it? The platform knows if
> its has funny boot image size limits, no?

The boot loader does not come with the kernel, so the platform cannot
know for sure.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds


Re: [PATCH v3 0/2] Ajust lockdep static allocations for sparc

2016-11-29 Thread Peter Zijlstra
On Tue, Nov 29, 2016 at 02:26:47PM +0100, Geert Uytterhoeven wrote:
> On Tue, Nov 29, 2016 at 1:29 PM, Peter Zijlstra  wrote:
> > On Tue, Nov 29, 2016 at 12:52:04PM +0100, Geert Uytterhoeven wrote:

> >> Not because of platforms with not limited memory, but because of platforms
> >> with boot loaders that have silly kernel size limitations, and start
> >> scribbling over the DTB or even theirselves when copying a large kernel 
> >> image.
> >
> > Right, that's the weird platforms clause above, and those can select the
> > option.
> 
> Their Kconfig files can select it. The users can't.
> What about making it visible depending on EXPERT?

Not understanding, why would a user ever need it? The platform knows if
its has funny boot image size limits, no?


renesas-drivers-2016-11-29-v4.9-rc7

2016-11-29 Thread Geert Uytterhoeven
I have pushed renesas-drivers-2016-11-29-v4.9-rc7 to
https://git.kernel.org/cgit/linux/kernel/git/geert/renesas-drivers.git

This tree is meant to ease development of platform support and drivers
for Renesas ARM SoCs. It is created by merging branches with driver code
submitted or planned for submission to maintainers into the development
branch of Simon Horman's renesas.git tree.

As we are currently focusing on a stable v4.9, no for-next branches
of various subsystem trees have been included.

Today's version is based on renesas-devel-20161128-v4.9-rc7.

Included branches with driver code:
  - clk-renesas-for-v4.11
  - sh-pfc-for-v4.11
  - topic/rcar-secondary-booting-in-debug-mode-v1-rebased1
  - topic/r8a7796-ravb-v1-rebased1
  - topic/r8a7796-dmac-driver-v2-rebased1
  - topic/r8a7796-dmac-dts-v2-rebased3~3
  - 
git://git.kernel.org/pub/scm/linux/kernel/git/horms/renesas.git#topic/sdr104-driver-v8
  - topic/vin-gen2-driver-v1-rebased2
  - topic/rcar-du-lvds-mode-selection-v1
  - topic/rcar-gen3-usb2-role-swap-v5
  - topic/dma-map-resource-v1
  - git://linuxtv.org/pinchartl/media.git#vsp1/histogram
  - https://git.ragnatech.se/linux#for-renesas-drivers
  - topic/ipmmu-multi-arch-v6
  - topic/r8a7795-ipmmu-v2-rebased4
  - topic/r8a7796-ipmmu-v1-rebased4
  - topic/r8a7795-ipmmu-integration-v1
  - topic/vin-gen2-dts-v1-rebased5
  - topic/vsp1-pa-improvements-v1-rebased1
  - topic/vsp1-writeback-v2-rebased2
  - git://linuxtv.org/pinchartl/media.git#drm/r8a7796/next
  - git://linuxtv.org/pinchartl/media.git#drm/next/lvds
  - topic/iommu-devel-du-rebased1
  - git://github.com/uli/kernel.git#r8a7795-hdmi-out-driver-v1
  - git://github.com/uli/kernel.git#r8a7795-hdmi-out-dts-v1
  - topic/r8a7795-es2-v1-rebased5
  - topic/r8a7796-64b-mem+ravb-prototype-v1-rebased4~1

As we're getting close to the opening of the merge window for v4.10, I
have also pushed renesas-drivers-next-2016-11-29-v4.9-rc7 to
https://git.kernel.org/cgit/linux/kernel/git/geert/renesas-drivers.git

This tree does include the following for-next branches of various
subsystem trees:
  - git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git#linux-next
  - git://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git#clk-next
  - 
git://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git#for-next
  - git://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git#for-next
  - git://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git#for-next
  - git://git.infradead.org/users/dedekind/l2-mtd-2.6.git#master
  - git://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git#master
  - git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git#tty-next
  - git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git#i2c/for-next
  - git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git#for-next
  - git://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can-next.git#master
  - git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git#usb-next
  - git://people.freedesktop.org/~airlied/linux#drm-next
  - git://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git#next
  - git://linuxtv.org/mchehab/media-next.git#master
  - git://git.kernel.org/pub/scm/linux/kernel/git/cjb/mmc.git#mmc-next
  - git://git.kernel.org/pub/scm/linux/kernel/git/ulfh/mmc.git#next
  - 
git://git.kernel.org/pub/scm/linux/kernel/git/thierry.reding/linux-pwm.git#for-next
  - git://git.linaro.org/people/daniel.lezcano/linux.git#clockevents/next
  - git://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git#testing/next
  - git://git.kernel.org/pub/scm/linux/kernel/git/djbw/dmaengine.git#next
  - git://git.infradead.org/users/vkoul/slave-dma.git#next
  - 
git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git#staging-next
  - git://git.armlinux.org.uk/~rmk/linux-arm.git#for-next
  - git://git.kernel.org/pub/scm/linux/kernel/git/rzhang/linux.git#next
  - git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regmap.git#for-next
  - git://git.infradead.org/users/jcooper/linux.git#irqchip/for-next
  - git://git.kernel.org/pub/scm/linux/kernel/git/tomba/linux.git#for-next
  - git://git.kernel.org/pub/scm/linux/kernel/git/tj/libata.git#for-next
  - git://git.infradead.org/battery-2.6.git#master
  - git://www.linux-watchdog.org/linux-watchdog-next.git#master
  - git://git.kernel.org/pub/scm/linux/kernel/git/arm/arm-soc.git#for-next
  - git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git#for-next
  - git://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git#for-next/core
  - git://anongit.freedesktop.org/drm-intel#topic/drm-misc
  - git://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git#next
  - git://git.kernel.org/pub/scm/linux/kernel/git/kishon/linux-phy.git#next
  - 
git://git.kernel.org/pub/scm/linux/kernel/git/evalenti/linux-soc-thermal.git#next

Included fixes:
  - net: phy: Fix use after free in phy_detach()

Included branches with driver code:
  - 
git://git.kernel.org/pub/scm/linux/kernel/git/wsa/l

Re: [PATCH v3 0/2] Ajust lockdep static allocations for sparc

2016-11-29 Thread Geert Uytterhoeven
On Tue, Nov 29, 2016 at 1:29 PM, Peter Zijlstra  wrote:
> On Tue, Nov 29, 2016 at 12:52:04PM +0100, Geert Uytterhoeven wrote:
>> > Nah, users don't need more senseless options. This is really only useful
>> > for dinky platforms or platforms with limited static image size (like
>> > sparc64).
>> >
>> > If you make this user selectable, someone will do, and then an endless
>> > stream of table not big enough warnings will be posted.
>> >
>> > Also, its only 4MB (IIRC), so who cares.
>>
>> I care :-)
>>
>> Not because of platforms with not limited memory, but because of platforms
>> with boot loaders that have silly kernel size limitations, and start
>> scribbling over the DTB or even theirselves when copying a large kernel 
>> image.
>
> Right, that's the weird platforms clause above, and those can select the
> option.

Their Kconfig files can select it. The users can't.
What about making it visible depending on EXPERT?

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds


[PATCH] pinctrl: sh-pfc: r8a7791: Add ADI pinconf support

2016-11-29 Thread Jacopo Mondi
Add pin configuration support for Gyro-ADC, named ADI on r8a7791 SoC.

The Gyro-ADC supports three different configurations:
a single ADC (adi and adi_b groups), 2 ADCs selectable through a single
channel select signal (adi_chsel1 and adi_chsel1_b groups), up to 4 ADCs
through 2 channel select signals (adi_chsel2 and adi_chsel2_b groups)
and up to 8 ADCs through 3 channel select signals (adi_chsel3 and
adi_chsel3_b groups)

Signed-off-by: Jacopo Mondi 
---

Compiled only, not tested with an actual ADC.

For reference only, these are the changes introduced by Geert's private
review of first sketch of this patch:
* separate ADI chsel in 3 overlapping groups:
  the user can now select how many pins to assign to channel selection
  function.

---
 drivers/pinctrl/sh-pfc/pfc-r8a7791.c | 86 
 1 file changed, 86 insertions(+)

diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a7791.c 
b/drivers/pinctrl/sh-pfc/pfc-r8a7791.c
index 7ca37c3..3898747 100644
--- a/drivers/pinctrl/sh-pfc/pfc-r8a7791.c
+++ b/drivers/pinctrl/sh-pfc/pfc-r8a7791.c
@@ -1691,6 +1691,72 @@ static const struct sh_pfc_pin pinmux_pins[] = {
PINMUX_GPIO_GP_ALL(),
 };
 
+/* - ADI  
*/
+static const unsigned int adi_pins[] = {
+   /* ADIDATA, ADICS, CLK*/
+   RCAR_GP_PIN(6, 24), RCAR_GP_PIN(6, 25), RCAR_GP_PIN(6, 26),
+};
+static const unsigned int adi_mux[] = {
+   /* ADIDATA, ADICS, CLK*/
+   ADIDATA_MARK, ADICS_SAMP_MARK, ADICLK_MARK,
+};
+static const unsigned int adi_chsel1_pins[] = {
+   /* CHS 0 */
+   RCAR_GP_PIN(6, 27),
+};
+static const unsigned int adi_chsel1_mux[] = {
+   /* CHS 0 */
+   ADICHS0_MARK, ADICHS1_MARK, ADICHS2_MARK,
+};
+static const unsigned int adi_chsel2_pins[] = {
+   /* CHS 0, 1 */
+   RCAR_GP_PIN(6, 27), RCAR_GP_PIN(6, 28),
+};
+static const unsigned int adi_chsel2_mux[] = {
+   /* CHS 0, 1 */
+   ADICHS0_MARK, ADICHS1_MARK, ADICHS2_MARK,
+};
+static const unsigned int adi_chsel3_pins[] = {
+   /* CHS 0, 1, 2 */
+   RCAR_GP_PIN(6, 27), RCAR_GP_PIN(6, 28), RCAR_GP_PIN(6, 29),
+};
+static const unsigned int adi_chsel3_mux[] = {
+   /* CHS 0, 1, 2 */
+   ADICHS0_MARK, ADICHS1_MARK, ADICHS2_MARK,
+};
+static const unsigned int adi_b_pins[] = {
+   /* ADIDATA B, ADICS B, CLK B */
+   RCAR_GP_PIN(5, 25), RCAR_GP_PIN(5, 26), RCAR_GP_PIN(5, 27),
+};
+static const unsigned int adi_b_mux[] = {
+   /* ADIDATA B, ADICS B, CLK B */
+   ADIDATA_B_MARK, ADICS_SAMP_B_MARK, ADICLK_B_MARK,
+};
+static const unsigned int adi_chsel1_b_pins[] = {
+   /* CHS B 0 */
+   RCAR_GP_PIN(5, 28),
+};
+static const unsigned int adi_chsel1_b_mux[] = {
+   /* CHS B 0 */
+   ADICHS0_B_MARK,
+};
+static const unsigned int adi_chsel2_b_pins[] = {
+   /* CHS B 0, 1 */
+   RCAR_GP_PIN(5, 28), RCAR_GP_PIN(5, 29),
+};
+static const unsigned int adi_chsel2_b_mux[] = {
+   /* CHS B 0, 1 */
+   ADICHS0_B_MARK, ADICHS1_B_MARK,
+};
+static const unsigned int adi_chsel3_b_pins[] = {
+   /* CHS B 0, 1, 2 */
+   RCAR_GP_PIN(5, 28), RCAR_GP_PIN(5, 29), RCAR_GP_PIN(5, 30),
+};
+static const unsigned int adi_chsel3_b_mux[] = {
+   /* CHS B 0, 1, 2 */
+   ADICHS0_B_MARK, ADICHS1_B_MARK, ADICHS2_B_MARK,
+};
+
 /* - Audio Clock  
*/
 static const unsigned int audio_clk_a_pins[] = {
/* CLK */
@@ -4343,6 +4409,14 @@ static const unsigned int vin2_clk_mux[] = {
 };
 
 static const struct sh_pfc_pin_group pinmux_groups[] = {
+   SH_PFC_PIN_GROUP(adi),
+   SH_PFC_PIN_GROUP(adi_chsel1),
+   SH_PFC_PIN_GROUP(adi_chsel2),
+   SH_PFC_PIN_GROUP(adi_chsel3),
+   SH_PFC_PIN_GROUP(adi_b),
+   SH_PFC_PIN_GROUP(adi_chsel1_b),
+   SH_PFC_PIN_GROUP(adi_chsel2_b),
+   SH_PFC_PIN_GROUP(adi_chsel3_b),
SH_PFC_PIN_GROUP(audio_clk_a),
SH_PFC_PIN_GROUP(audio_clk_b),
SH_PFC_PIN_GROUP(audio_clk_b_b),
@@ -4687,6 +4761,17 @@ static const struct sh_pfc_pin_group pinmux_groups[] = {
SH_PFC_PIN_GROUP(vin2_clk),
 };
 
+static const char * const adi_groups[] = {
+   "adi",
+   "adi_chsel1",
+   "adi_chsel2",
+   "adi_chsel3",
+   "adi_b",
+   "adi_chsel1_b",
+   "adi_chsel2_b",
+   "adi_chsel3_b",
+};
+
 static const char * const audio_clk_groups[] = {
"audio_clk_a",
"audio_clk_b",
@@ -5192,6 +5277,7 @@ static const char * const vin2_groups[] = {
 };
 
 static const struct sh_pfc_function pinmux_functions[] = {
+   SH_PFC_FUNCTION(adi),
SH_PFC_FUNCTION(audio_clk),
SH_PFC_FUNCTION(avb),
SH_PFC_FUNCTION(can0),
-- 
2.7.4



Re: [PATCH v3 0/2] Ajust lockdep static allocations for sparc

2016-11-29 Thread Peter Zijlstra
On Tue, Nov 29, 2016 at 01:29:07PM +0100, Peter Zijlstra wrote:
> > BTW, is there any particular reason these huge arrays are in BSS, and not
> > allocated dynamically? That would solve my problems as well...
> 
> Is there a memory allocator available before _any_ locks are used, and
> that itself also doesn't use locks?

And if that is, the platform could use that to allocate .bss, no?


Re: [PATCH v3 0/2] Ajust lockdep static allocations for sparc

2016-11-29 Thread Peter Zijlstra
On Tue, Nov 29, 2016 at 12:52:04PM +0100, Geert Uytterhoeven wrote:
> > Nah, users don't need more senseless options. This is really only useful
> > for dinky platforms or platforms with limited static image size (like
> > sparc64).
> >
> > If you make this user selectable, someone will do, and then an endless
> > stream of table not big enough warnings will be posted.
> >
> > Also, its only 4MB (IIRC), so who cares.
> 
> I care :-)
> 
> Not because of platforms with not limited memory, but because of platforms
> with boot loaders that have silly kernel size limitations, and start
> scribbling over the DTB or even theirselves when copying a large kernel image.

Right, that's the weird platforms clause above, and those can select the
option.

> BTW, is there any particular reason these huge arrays are in BSS, and not
> allocated dynamically? That would solve my problems as well...

Is there a memory allocator available before _any_ locks are used, and
that itself also doesn't use locks?


Re: [PATCH v3 0/2] Ajust lockdep static allocations for sparc

2016-11-29 Thread Geert Uytterhoeven
Hi Peter,

On Tue, Nov 29, 2016 at 12:41 PM, Peter Zijlstra  wrote:
> On Tue, Nov 29, 2016 at 12:14:48PM +0100, Geert Uytterhoeven wrote:
>> CC linux-renesas-soc
>>
>> On Tue, Sep 27, 2016 at 9:33 PM, Babu Moger  wrote:
>> > These patches limit the static allocations for lockdep data structures
>> > used for debugging locking correctness. For sparc, all the kernel's code,
>> > data, and bss, must have locked translations in the TLB so that we don't
>> > get TLB misses on kernel code and data. Current sparc chips have 8 TLB
>> > entries available that may be locked down, and with a 4mb page size,
>> > this gives a maximum of 32MB. With PROVE_LOCKING we could go over this
>> > limit and cause system boot-up problems. These patches limit the static
>> > allocations so that everything fits in current required size limit.
>> >
>> > patch 1 : Adds new config parameter CONFIG_PROVE_LOCKING_SMALL
>> > Patch 2 : Adjusts the sizes based on the new config parameter
>>
>> Cool, this is also useful on other platforms!
>> E.g. on r8a7791/koelsch, I cannot boot an shmobile_defconfig plus
>> CONFIG_PROVE_LOCKING kernel.
>> Forcing CONFIG_PROVE_LOCKING_SMALL=y in Kconfig fixes that.
>>
>> Should it become a user-selectable symbol?
>
> Nah, users don't need more senseless options. This is really only useful
> for dinky platforms or platforms with limited static image size (like
> sparc64).
>
> If you make this user selectable, someone will do, and then an endless
> stream of table not big enough warnings will be posted.
>
> Also, its only 4MB (IIRC), so who cares.

I care :-)

Not because of platforms with not limited memory, but because of platforms
with boot loaders that have silly kernel size limitations, and start
scribbling over the DTB or even theirselves when copying a large kernel image.

BTW, is there any particular reason these huge arrays are in BSS, and not
allocated dynamically? That would solve my problems as well...

Thanks!

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds


Re: [PATCH v3 0/2] Ajust lockdep static allocations for sparc

2016-11-29 Thread Peter Zijlstra
On Tue, Nov 29, 2016 at 12:14:48PM +0100, Geert Uytterhoeven wrote:
> CC linux-renesas-soc
> 
> On Tue, Sep 27, 2016 at 9:33 PM, Babu Moger  wrote:
> > These patches limit the static allocations for lockdep data structures
> > used for debugging locking correctness. For sparc, all the kernel's code,
> > data, and bss, must have locked translations in the TLB so that we don't
> > get TLB misses on kernel code and data. Current sparc chips have 8 TLB
> > entries available that may be locked down, and with a 4mb page size,
> > this gives a maximum of 32MB. With PROVE_LOCKING we could go over this
> > limit and cause system boot-up problems. These patches limit the static
> > allocations so that everything fits in current required size limit.
> >
> > patch 1 : Adds new config parameter CONFIG_PROVE_LOCKING_SMALL
> > Patch 2 : Adjusts the sizes based on the new config parameter
> 
> Cool, this is also useful on other platforms!
> E.g. on r8a7791/koelsch, I cannot boot an shmobile_defconfig plus
> CONFIG_PROVE_LOCKING kernel.
> Forcing CONFIG_PROVE_LOCKING_SMALL=y in Kconfig fixes that.
> 
> Should it become a user-selectable symbol?

Nah, users don't need more senseless options. This is really only useful
for dinky platforms or platforms with limited static image size (like
sparc64).

If you make this user selectable, someone will do, and then an endless
stream of table not big enough warnings will be posted.

Also, its only 4MB (IIRC), so who cares.


Re: [PATCH v3 0/2] Ajust lockdep static allocations for sparc

2016-11-29 Thread Geert Uytterhoeven
CC linux-renesas-soc

On Tue, Sep 27, 2016 at 9:33 PM, Babu Moger  wrote:
> These patches limit the static allocations for lockdep data structures
> used for debugging locking correctness. For sparc, all the kernel's code,
> data, and bss, must have locked translations in the TLB so that we don't
> get TLB misses on kernel code and data. Current sparc chips have 8 TLB
> entries available that may be locked down, and with a 4mb page size,
> this gives a maximum of 32MB. With PROVE_LOCKING we could go over this
> limit and cause system boot-up problems. These patches limit the static
> allocations so that everything fits in current required size limit.
>
> patch 1 : Adds new config parameter CONFIG_PROVE_LOCKING_SMALL
> Patch 2 : Adjusts the sizes based on the new config parameter

Cool, this is also useful on other platforms!
E.g. on r8a7791/koelsch, I cannot boot an shmobile_defconfig plus
CONFIG_PROVE_LOCKING kernel.
Forcing CONFIG_PROVE_LOCKING_SMALL=y in Kconfig fixes that.

Should it become a user-selectable symbol?

> v2-> v3:
>Some more comments from Sam Ravnborg and Peter Zijlstra.
>Defined PROVE_LOCKING_SMALL as invisible and moved the selection to
>arch/sparc/Kconfig.
>
> v1-> v2:
>As suggested by Peter Zijlstra, keeping the default as is.
>Introduced new config variable CONFIG_PROVE_LOCKING_SMALL
>to handle sparc specific case.
>
> v0:
>Initial revision.
>
> Babu Moger (2):
>   config: Adding the new config parameter CONFIG_PROVE_LOCKING_SMALL
> for sparc
>   lockdep: Limit static allocations if PROVE_LOCKING_SMALL is defined
>
>  arch/sparc/Kconfig |1 +
>  kernel/locking/lockdep_internals.h |   20 +---
>  lib/Kconfig.debug  |3 +++
>  3 files changed, 21 insertions(+), 3 deletions(-)

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds


Re: [PATCH v3 04/13] drm: bridge: Detach bridge from encoder at encoder cleanup time

2016-11-29 Thread Archit Taneja



On 11/29/2016 02:34 PM, Laurent Pinchart wrote:

Most drivers that use bridges forgot to detach them at cleanup time.
Instead of fixing them one by one, detach the bridge in the core
drm_encoder_cleanup() function.

Signed-off-by: Laurent Pinchart 
---
 drivers/gpu/drm/drm_encoder.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/drm_encoder.c b/drivers/gpu/drm/drm_encoder.c
index 5c067719164d..9c1f99646e0d 100644
--- a/drivers/gpu/drm/drm_encoder.c
+++ b/drivers/gpu/drm/drm_encoder.c
@@ -164,6 +164,9 @@ void drm_encoder_cleanup(struct drm_encoder *encoder)
 * the indices on the drm_encoder after us in the encoder_list.
 */

+   if (encoder->bridge)
+   drm_bridge_detach(encoder->bridge);


This would require the kms driver to still detach the remaining
n - 1 bridges in a possible chain. We could probably detach all of
them here, or maybe leave detaching of all to the kms driver, and just
report a warning here.

Archit


+
drm_modeset_lock_all(dev);
drm_mode_object_unregister(dev, &encoder->base);
kfree(encoder->name);



--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project


Re: [PATCH v3 09/13] drm: Add encoder_type field to the drm_bridge structure

2016-11-29 Thread Daniel Vetter
On Tue, Nov 29, 2016 at 11:58:44AM +0200, Laurent Pinchart wrote:
> On Tuesday 29 Nov 2016 10:56:53 Daniel Vetter wrote:
> > On Tue, Nov 29, 2016 at 11:04:39AM +0200, Laurent Pinchart wrote:
> > > The drm_bridge object models on- or off-chip hardware encoders and
> > > provide an abstract control API to display drivers. In order to help
> > > display drivers creating the right kind of drm_encoder object, expose
> > > the type of the hardware encoder associated with each bridge.
> > > 
> > > Signed-off-by: Laurent Pinchart
> > > 
> > 
> > DRM_MODE_ENCODER_BRIDGE. Problem solved, because in reality no one cares
> > one iota about the encoder type.
> 
> It's exposed to userspace though, are you 100% sure we won't break anything ?

We've added DP, DSI, DPMST and DPI encoder types thus far, no one
screamed. We should be fine I think. Connector types are a bit different,
userspace (at least X) wants to pretty-print them for xrandr names.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch


Re: [PATCH v3 03/13] drm: bridge: Link encoder and bridge in core code

2016-11-29 Thread Archit Taneja



On 11/29/2016 02:34 PM, Laurent Pinchart wrote:

Instead of linking encoders and bridges in every driver (and getting it
wrong half of the time, as many drivers forget to set the drm_bridge
encoder pointer), do so in core code. The drm_bridge_attach() function
needs the encoder and optional previous bridge to perform that task,
update all the callers.

Signed-off-by: Laurent Pinchart 
---
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c   |  4 +-
 drivers/gpu/drm/bridge/analogix/analogix_dp_core.c |  4 +-
 drivers/gpu/drm/bridge/dw-hdmi.c   |  3 +-
 drivers/gpu/drm/drm_bridge.c   | 46 --
 drivers/gpu/drm/drm_simple_kms_helper.c|  4 +-
 drivers/gpu/drm/exynos/exynos_dp.c |  5 +--
 drivers/gpu/drm/exynos/exynos_drm_dsi.c|  6 +--
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c  |  5 +--
 drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c   |  5 +--
 drivers/gpu/drm/imx/imx-ldb.c  |  6 +--
 drivers/gpu/drm/imx/parallel-display.c |  4 +-
 drivers/gpu/drm/mediatek/mtk_dpi.c |  8 ++--
 drivers/gpu/drm/mediatek/mtk_dsi.c | 24 ++-
 drivers/gpu/drm/mediatek/mtk_hdmi.c| 11 +++---
 drivers/gpu/drm/msm/dsi/dsi_manager.c  | 17 +---
 drivers/gpu/drm/msm/edp/edp_bridge.c   |  2 +-
 drivers/gpu/drm/msm/hdmi/hdmi_bridge.c |  2 +-
 drivers/gpu/drm/rcar-du/rcar_du_hdmienc.c  |  5 +--
 drivers/gpu/drm/sti/sti_dvo.c  |  3 +-
 drivers/gpu/drm/sti/sti_hda.c  |  3 +-
 drivers/gpu/drm/sti/sti_hdmi.c |  3 +-
 drivers/gpu/drm/sun4i/sun4i_rgb.c  | 13 +++---
 include/drm/drm_bridge.h   |  3 +-
 23 files changed, 83 insertions(+), 103 deletions(-)

diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c 
b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c
index 6119b5085501..e7799b6ee829 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c
@@ -230,9 +230,7 @@ static int atmel_hlcdc_attach_endpoint(struct drm_device 
*dev,
of_node_put(np);

if (bridge) {
-   output->encoder.bridge = bridge;
-   bridge->encoder = &output->encoder;
-   ret = drm_bridge_attach(dev, bridge);
+   ret = drm_bridge_attach(&output->encoder, bridge, NULL);
if (!ret)
return 0;
}
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c 
b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
index 6e0447f329a2..1835f1fdad19 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
@@ -1227,12 +1227,10 @@ static int analogix_dp_create_bridge(struct drm_device 
*drm_dev,

dp->bridge = bridge;

-   dp->encoder->bridge = bridge;
bridge->driver_private = dp;
-   bridge->encoder = dp->encoder;
bridge->funcs = &analogix_dp_bridge_funcs;

-   ret = drm_bridge_attach(drm_dev, bridge);
+   ret = drm_bridge_attach(dp->encoder, bridge, NULL);
if (ret) {
DRM_ERROR("failed to attach drm bridge\n");
return -EINVAL;
diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c b/drivers/gpu/drm/bridge/dw-hdmi.c
index b71088dab268..432e0e3fff72 100644
--- a/drivers/gpu/drm/bridge/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/dw-hdmi.c
@@ -1841,13 +1841,12 @@ static int dw_hdmi_register(struct drm_device *drm, 
struct dw_hdmi *hdmi)
hdmi->bridge = bridge;
bridge->driver_private = hdmi;
bridge->funcs = &dw_hdmi_bridge_funcs;
-   ret = drm_bridge_attach(drm, bridge);
+   ret = drm_bridge_attach(encoder, bridge, NULL);
if (ret) {
DRM_ERROR("Failed to initialize bridge with drm\n");
return -EINVAL;
}

-   encoder->bridge = bridge;
hdmi->connector.polled = DRM_CONNECTOR_POLL_HPD;

drm_connector_helper_add(&hdmi->connector,
diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
index 0ee052b7c21a..850bd6509ef1 100644
--- a/drivers/gpu/drm/drm_bridge.c
+++ b/drivers/gpu/drm/drm_bridge.c
@@ -26,6 +26,7 @@
 #include 

 #include 
+#include 

 /**
  * DOC: overview
@@ -92,32 +93,53 @@ void drm_bridge_remove(struct drm_bridge *bridge)
 EXPORT_SYMBOL(drm_bridge_remove);

 /**
- * drm_bridge_attach - associate given bridge to our DRM device
+ * drm_bridge_attach - attach the bridge to an encoder's chain
  *
- * @dev: DRM device
- * @bridge: bridge control structure
+ * @encoder: DRM encoder
+ * @bridge: bridge to attach
+ * @previous: previous bridge in the chain (optional)
  *
- * Called by a kms driver to link one of our encoder/bridge to the given
- * bridge.
+ * Called by a kms driver to link the bridge to an encoder's chain. The 
previous
+ * arg

Re: [PATCH] extcon: Split out the extcon APIs for extcon provider driver

2016-11-29 Thread Charles Keepax
On Tue, Nov 29, 2016 at 07:02:23PM +0900, Chanwoo Choi wrote:
> This patchs split out the extcon APIs of extcon provider driver in order to
> prevent the direct access of struct extcon_dev by extcon consumer driver.
> The extcon consumer driver don't need to handle the extcon provider APIs.
> 
> The extcon subsystem has two type of extcon drivers as following:
> - extcon provider driver
> : Detect the external connector and identify the state/property of
>   each external connector. And it send the notification to synchronize
>   the information between provider and consumer driver.
> - extcon consumer driver
> : Receive the notifcation from extcon provider driver. When receving the noti,
>   it can get the both state and property of specific external connector.
> 
> Cc: Myungjoo Ham 
> Cc: Chen-Yu Tsai 
> Cc: Krzysztof Kozlowski 
> Cc: Yoshihiro Shimoda 
> Cc: Kishon Vijay Abraham I 
> Cc: Maxime Ripard 
> Cc: Felipe Balbi 
> Cc: Greg Kroah-Hartman 
> Cc: Chris Zhong 
> Cc: Roger Quadros 
> Cc: Charles Keepax 
> Cc: patc...@opensource.wolfsonmicro.com
> Cc: linux-renesas-soc@vger.kernel.org
> Cc: linux-arm-ker...@lists.infradead.org
> Cc: linux-...@vger.kernel.org
> Signed-off-by: Chanwoo Choi 
> ---
>  drivers/extcon/devres.c|   2 +-
>  drivers/extcon/extcon-adc-jack.c   |   2 +-
>  drivers/extcon/extcon-arizona.c|   2 +-

For the Arizona bit:

Acked-by: Charles Keepax 

Thanks,
Charles


Re: [PATCH v3 03/13] drm: bridge: Link encoder and bridge in core code

2016-11-29 Thread Daniel Vetter
On Tue, Nov 29, 2016 at 11:43:19AM +0200, Laurent Pinchart wrote:
> Hi Daniel,
> 
> On Tuesday 29 Nov 2016 10:35:24 Daniel Vetter wrote:
> > On Tue, Nov 29, 2016 at 11:04:33AM +0200, Laurent Pinchart wrote:
> > > Instead of linking encoders and bridges in every driver (and getting it
> > > wrong half of the time, as many drivers forget to set the drm_bridge
> > > encoder pointer), do so in core code. The drm_bridge_attach() function
> > > needs the encoder and optional previous bridge to perform that task,
> > > update all the callers.
> > > 
> > > Signed-off-by: Laurent Pinchart
> > > 
> > > ---
> > > 
> > >  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c   |  4 +-
> > >  drivers/gpu/drm/bridge/analogix/analogix_dp_core.c |  4 +-
> > >  drivers/gpu/drm/bridge/dw-hdmi.c   |  3 +-
> > >  drivers/gpu/drm/drm_bridge.c   | 46 +
> > >  drivers/gpu/drm/drm_simple_kms_helper.c|  4 +-
> > >  drivers/gpu/drm/exynos/exynos_dp.c |  5 +--
> > >  drivers/gpu/drm/exynos/exynos_drm_dsi.c|  6 +--
> > >  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c  |  5 +--
> > >  drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c   |  5 +--
> > >  drivers/gpu/drm/imx/imx-ldb.c  |  6 +--
> > >  drivers/gpu/drm/imx/parallel-display.c |  4 +-
> > >  drivers/gpu/drm/mediatek/mtk_dpi.c |  8 ++--
> > >  drivers/gpu/drm/mediatek/mtk_dsi.c | 24 ++-
> > >  drivers/gpu/drm/mediatek/mtk_hdmi.c| 11 +++---
> > >  drivers/gpu/drm/msm/dsi/dsi_manager.c  | 17 +---
> > >  drivers/gpu/drm/msm/edp/edp_bridge.c   |  2 +-
> > >  drivers/gpu/drm/msm/hdmi/hdmi_bridge.c |  2 +-
> > >  drivers/gpu/drm/rcar-du/rcar_du_hdmienc.c  |  5 +--
> > >  drivers/gpu/drm/sti/sti_dvo.c  |  3 +-
> > >  drivers/gpu/drm/sti/sti_hda.c  |  3 +-
> > >  drivers/gpu/drm/sti/sti_hdmi.c |  3 +-
> > >  drivers/gpu/drm/sun4i/sun4i_rgb.c  | 13 +++---
> > >  include/drm/drm_bridge.h   |  3 +-
> > >  23 files changed, 83 insertions(+), 103 deletions(-)
> 
> [snip]
> 
> > > diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
> > > index 0ee052b7c21a..850bd6509ef1 100644
> > > --- a/drivers/gpu/drm/drm_bridge.c
> > > +++ b/drivers/gpu/drm/drm_bridge.c
> > > @@ -26,6 +26,7 @@
> > > 
> > >  #include 
> > >  #include 
> > > +#include 
> > > 
> > >  /**
> > >   * DOC: overview
> > > @@ -92,32 +93,53 @@ void drm_bridge_remove(struct drm_bridge *bridge)
> > >  EXPORT_SYMBOL(drm_bridge_remove);
> > >  
> > >  /**
> > > - * drm_bridge_attach - associate given bridge to our DRM device
> > > + * drm_bridge_attach - attach the bridge to an encoder's chain
> > >   *
> > > - * @dev: DRM device
> > > - * @bridge: bridge control structure
> > > + * @encoder: DRM encoder
> > > + * @bridge: bridge to attach
> > > + * @previous: previous bridge in the chain (optional)
> > >   *
> > > - * Called by a kms driver to link one of our encoder/bridge to the given
> > > - * bridge.
> > > + * Called by a kms driver to link the bridge to an encoder's chain. The
> > > previous
> > > + * argument specifies the previous bridge in the chain. If NULL, the
> > > bridge is
> > > + * linked directly at the encoder's output. Otherwise it is linked at the
> > > + * previous bridge's output.
> > >   *
> > > - * Note that setting up links between the bridge and our encoder/bridge
> > > - * objects needs to be handled by the kms driver itself.
> > > + * If non-NULL the previous bridge must be already attached by a call to
> > > this
> > > + * function.
> > >   *
> > >   * RETURNS:
> > >   * Zero on success, error code on failure
> > >   */
> > > -int drm_bridge_attach(struct drm_device *dev, struct drm_bridge *bridge)
> > > +int drm_bridge_attach(struct drm_encoder *encoder, struct drm_bridge
> > > *bridge,
> > > +   struct drm_bridge *previous)
> > >  {
> > > - if (!dev || !bridge)
> > > + int ret;
> > > +
> > > + if (!encoder || !bridge)
> > > + return -EINVAL;
> > > +
> > > + if (previous && (!previous->dev || previous->encoder != encoder))
> > >   return -EINVAL;
> > 
> > Not sure we want to allow setting both encoder and bridge for chaining.
> > I'd kinda expect that for chained use-case the bridge doesn't care that
> > much who started the chain. And if not we can always create a helper to
> > look up the drm_encoder.
> 
> As bridge drivers currently create connectors (I'd like to change that at 
> some 
> point, but one thing at a time) they need to call 
> drm_mode_connector_attach_encoder() and thus need to have access to the 
> drm_encoder object at the beginning of the chain. The drm_bridge structure 
> has 
> an encoder field, it seems to me that the easiest is to always populate it, 
> regardless of the position of the bridge in the chain.

I m

[PATCH] extcon: Split out the extcon APIs for extcon provider driver

2016-11-29 Thread Chanwoo Choi
This patchs split out the extcon APIs of extcon provider driver in order to
prevent the direct access of struct extcon_dev by extcon consumer driver.
The extcon consumer driver don't need to handle the extcon provider APIs.

The extcon subsystem has two type of extcon drivers as following:
- extcon provider driver
: Detect the external connector and identify the state/property of
  each external connector. And it send the notification to synchronize
  the information between provider and consumer driver.
- extcon consumer driver
: Receive the notifcation from extcon provider driver. When receving the noti,
  it can get the both state and property of specific external connector.

Cc: Myungjoo Ham 
Cc: Chen-Yu Tsai 
Cc: Krzysztof Kozlowski 
Cc: Yoshihiro Shimoda 
Cc: Kishon Vijay Abraham I 
Cc: Maxime Ripard 
Cc: Felipe Balbi 
Cc: Greg Kroah-Hartman 
Cc: Chris Zhong 
Cc: Roger Quadros 
Cc: Charles Keepax 
Cc: patc...@opensource.wolfsonmicro.com
Cc: linux-renesas-soc@vger.kernel.org
Cc: linux-arm-ker...@lists.infradead.org
Cc: linux-...@vger.kernel.org
Signed-off-by: Chanwoo Choi 
---
 drivers/extcon/devres.c|   2 +-
 drivers/extcon/extcon-adc-jack.c   |   2 +-
 drivers/extcon/extcon-arizona.c|   2 +-
 drivers/extcon/extcon-axp288.c |   2 +-
 drivers/extcon/extcon-gpio.c   |   2 +-
 drivers/extcon/extcon-max14577.c   |   2 +-
 drivers/extcon/extcon-max3355.c|   2 +-
 drivers/extcon/extcon-max77693.c   |   2 +-
 drivers/extcon/extcon-max77843.c   |   2 +-
 drivers/extcon/extcon-max8997.c|   2 +-
 drivers/extcon/extcon-palmas.c |   1 +
 drivers/extcon/extcon-qcom-spmi-misc.c |   2 +-
 drivers/extcon/extcon-rt8973a.c|   2 +-
 drivers/extcon/extcon-sm5502.c |   2 +-
 drivers/extcon/extcon-usb-gpio.c   |   2 +-
 drivers/extcon/extcon.c|   2 +-
 drivers/phy/phy-rcar-gen3-usb2.c   |   2 +-
 drivers/phy/phy-sun4i-usb.c|   2 +-
 drivers/power/supply/qcom_smbb.c   |   2 +-
 drivers/usb/phy/phy-tahvo.c|   2 +-
 include/linux/extcon-provider.h| 262 +
 include/linux/extcon.h | 168 +
 22 files changed, 286 insertions(+), 183 deletions(-)
 create mode 100644 include/linux/extcon-provider.h

diff --git a/drivers/extcon/devres.c b/drivers/extcon/devres.c
index e686acd1c459..1b7dce13b21b 100644
--- a/drivers/extcon/devres.c
+++ b/drivers/extcon/devres.c
@@ -14,7 +14,7 @@
  * GNU General Public License for more details.
  */
 
-#include 
+#include 
 
 static int devm_extcon_dev_match(struct device *dev, void *res, void *data)
 {
diff --git a/drivers/extcon/extcon-adc-jack.c b/drivers/extcon/extcon-adc-jack.c
index bc538708c753..0882813a95f9 100644
--- a/drivers/extcon/extcon-adc-jack.c
+++ b/drivers/extcon/extcon-adc-jack.c
@@ -26,7 +26,7 @@
 #include 
 #include 
 #include 
-#include 
+#include 
 
 /**
  * struct adc_jack_data - internal data for adc_jack device driver
diff --git a/drivers/extcon/extcon-arizona.c b/drivers/extcon/extcon-arizona.c
index aeab47df9233..292878714ed9 100644
--- a/drivers/extcon/extcon-arizona.c
+++ b/drivers/extcon/extcon-arizona.c
@@ -27,7 +27,7 @@
 #include 
 #include 
 #include 
-#include 
+#include 
 
 #include 
 
diff --git a/drivers/extcon/extcon-axp288.c b/drivers/extcon/extcon-axp288.c
index 42f41e808292..f5b3687d29e8 100644
--- a/drivers/extcon/extcon-axp288.c
+++ b/drivers/extcon/extcon-axp288.c
@@ -23,7 +23,7 @@
 #include 
 #include 
 #include 
-#include 
+#include 
 #include 
 #include 
 #include 
diff --git a/drivers/extcon/extcon-gpio.c b/drivers/extcon/extcon-gpio.c
index ebed22f22d75..ab770adcca7e 100644
--- a/drivers/extcon/extcon-gpio.c
+++ b/drivers/extcon/extcon-gpio.c
@@ -17,7 +17,7 @@
  * GNU General Public License for more details.
  */
 
-#include 
+#include 
 #include 
 #include 
 #include 
diff --git a/drivers/extcon/extcon-max14577.c b/drivers/extcon/extcon-max14577.c
index 12e26c4e7763..9a7524ebf698 100644
--- a/drivers/extcon/extcon-max14577.c
+++ b/drivers/extcon/extcon-max14577.c
@@ -23,7 +23,7 @@
 #include 
 #include 
 #include 
-#include 
+#include 
 
 #defineDELAY_MS_DEFAULT17000   /* unit: 
millisecond */
 
diff --git a/drivers/extcon/extcon-max3355.c b/drivers/extcon/extcon-max3355.c
index 533e16a952b8..0aa410836f4e 100644
--- a/drivers/extcon/extcon-max3355.c
+++ b/drivers/extcon/extcon-max3355.c
@@ -9,7 +9,7 @@
  * may be copied, distributed, and modified under those terms.
  */
 
-#include 
+#include 
 #include 
 #include 
 #include 
diff --git a/drivers/extcon/extcon-max77693.c b/drivers/extcon/extcon-max77693.c
index 68dbcb814b2f..d49339591cb6 100644
--- a/drivers/extcon/extcon-max77693.c
+++ b/drivers/extcon/extcon-max77693.c
@@ -26,7 +26,7 @@
 #include 
 #include 
 #include 
-#include 
+#include 
 #include 
 #include 
 
diff --git a/drivers/extcon/extcon-max77843.c b/drivers/extcon/extcon-max77843.c
index 

Re: [PATCH v3 09/13] drm: Add encoder_type field to the drm_bridge structure

2016-11-29 Thread Laurent Pinchart
On Tuesday 29 Nov 2016 10:56:53 Daniel Vetter wrote:
> On Tue, Nov 29, 2016 at 11:04:39AM +0200, Laurent Pinchart wrote:
> > The drm_bridge object models on- or off-chip hardware encoders and
> > provide an abstract control API to display drivers. In order to help
> > display drivers creating the right kind of drm_encoder object, expose
> > the type of the hardware encoder associated with each bridge.
> > 
> > Signed-off-by: Laurent Pinchart
> > 
> 
> DRM_MODE_ENCODER_BRIDGE. Problem solved, because in reality no one cares
> one iota about the encoder type.

It's exposed to userspace though, are you 100% sure we won't break anything ?

-- 
Regards,

Laurent Pinchart



Re: [PATCH v3 09/13] drm: Add encoder_type field to the drm_bridge structure

2016-11-29 Thread Daniel Vetter
On Tue, Nov 29, 2016 at 11:04:39AM +0200, Laurent Pinchart wrote:
> The drm_bridge object models on- or off-chip hardware encoders and
> provide an abstract control API to display drivers. In order to help
> display drivers creating the right kind of drm_encoder object, expose
> the type of the hardware encoder associated with each bridge.
> 
> Signed-off-by: Laurent Pinchart 

DRM_MODE_ENCODER_BRIDGE. Problem solved, because in reality no one cares
one iota about the encoder type.
-Daniel

> ---
> Changes since v1:
> 
> - Clarify documentation
> ---
>  include/drm/drm_bridge.h | 8 
>  1 file changed, 8 insertions(+)
> 
> diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
> index 94e5ee96b3b5..0643b6ce27de 100644
> --- a/include/drm/drm_bridge.h
> +++ b/include/drm/drm_bridge.h
> @@ -194,6 +194,14 @@ struct drm_bridge {
>  #endif
>   struct list_head list;
>  
> + /**
> +  * @encoder_type:
> +  *
> +  * Type of the hardware encoder modeled by the bridge as one of the
> +  * DRM_MODE_ENCODER_* types. For the last bridge in the chain this will
> +  * usually be identical to the bridge's &drm_encoder encoder_type.
> +  */
> + int encoder_type;
>   const struct drm_bridge_funcs *funcs;
>   void *driver_private;
>  };
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch


Re: [PATCH v3 06/13] drm: bridge: Add LVDS encoder driver

2016-11-29 Thread Daniel Vetter
On Tue, Nov 29, 2016 at 11:04:36AM +0200, Laurent Pinchart wrote:
> The LVDS encoder driver is a DRM bridge driver that supports the
> parallel to LVDS encoders that don't require any configuration. The
> driver thus doesn't interact with the device, but creates an LVDS
> connector for the panel and exposes its size and timing based on
> information retrieved from DT.
> 
> Signed-off-by: Laurent Pinchart 

Since it's 100% dummy, why put LVDS into the name? This little thing here
could be our generic "wrap drm_panel and attach it to a chain" helper. So
what about calling this _The_ drm_panel_bridge, and also linking it into
docs to feature it a bit more prominently.

I came up with this because I spotted some refactoring belows for building
this helper, until I realized that this driver _is_ the helper I think we
want ;-) Only thing missing is an exported function to instantiate a
bridge with just a drm_panel as the parameter. And putting it into the
drm_kms_helper.ko module.

> +static enum drm_connector_status
> +lvds_connector_detect(struct drm_connector *connector, bool force)
> +{
> + return connector_status_connected;
> +}

We have piles of this exact dummy callback all over, maybe make it the
default and rip them all out?
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch


Re: [PATCH v3 07/13] drm: bridge: vga-dac: Add adi,adv7123 compatible string

2016-11-29 Thread Maxime Ripard
On Tue, Nov 29, 2016 at 11:04:37AM +0200, Laurent Pinchart wrote:
> The ADV7123 is a transparent VGA DAC. Unlike dumb VGA DACs it can be
> controlled through a power save pin, and requires a power supply.
> However, on most boards where the device is used neither the power save
> signal nor the power supply are controllable.
> 
> To avoid developing a separate device-specific driver add an
> "adi,adv7123" compatible entry to the dumb-vga-dac driver. This will
> allow supporting most ADV7123-based boards easily, while allowing future
> development of an adv7123 driver when needed without breaking backward
> compatibility.
> 
> Signed-off-by: Laurent Pinchart 

Acked-by: Maxime Ripard 

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com


signature.asc
Description: PGP signature


Re: [PATCH v3 04/13] drm: bridge: Detach bridge from encoder at encoder cleanup time

2016-11-29 Thread Daniel Vetter
On Tue, Nov 29, 2016 at 11:04:34AM +0200, Laurent Pinchart wrote:
> Most drivers that use bridges forgot to detach them at cleanup time.
> Instead of fixing them one by one, detach the bridge in the core
> drm_encoder_cleanup() function.
> 
> Signed-off-by: Laurent Pinchart 
> ---
>  drivers/gpu/drm/drm_encoder.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_encoder.c b/drivers/gpu/drm/drm_encoder.c
> index 5c067719164d..9c1f99646e0d 100644
> --- a/drivers/gpu/drm/drm_encoder.c
> +++ b/drivers/gpu/drm/drm_encoder.c
> @@ -164,6 +164,9 @@ void drm_encoder_cleanup(struct drm_encoder *encoder)
>* the indices on the drm_encoder after us in the encoder_list.
>*/
>  
> + if (encoder->bridge)
> + drm_bridge_detach(encoder->bridge);

Means we bake in drm_bridge much more as a core thing, but I guess that's
ok.

But there's 3 callers of drm_bridge_detach outside of the drm core, can't
we remove them and drop the EXPORT_SYMBOL for drm_bridge_detach? What's
it still needed for?

I think that cleanup should done in this patch here - drm_bridge_detach
WARN_ONs if the bridge is already detached ...
-Daniel

> +
>   drm_modeset_lock_all(dev);
>   drm_mode_object_unregister(dev, &encoder->base);
>   kfree(encoder->name);
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch


Re: [PATCH v3] iommu/ipmmu-vmsa: Add r8a7796 DT binding

2016-11-29 Thread Geert Uytterhoeven
On Wed, Nov 23, 2016 at 3:40 PM, Magnus Damm  wrote:
> From: Magnus Damm 
>
> Update the IPMMU DT binding documentation to include the r8a7796 compat
> string for R-Car M3-W.
>
> Signed-off-by: Magnus Damm 
> Acked-by: Laurent Pinchart 
> Acked-by: Rob Herring 
> Acked-by: Simon Horman 

Acked-by: Geert Uytterhoeven 

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds


Re: [PATCH] pinctrl: sh-pfc: r8a7795: Support none GPIO pins bias setting

2016-11-29 Thread Geert Uytterhoeven
On Thu, Nov 17, 2016 at 4:26 PM, Niklas Söderlund
 wrote:
> There are pins on the r8a7795 which are not part of a GPIO bank nor
> can be muxed between different functions. They do however allow for the
> bias to be configured. Add those pins to the list of pins and
> to the bias configuration array.
>
> The pins can now be referred to in DT by function names and their bias
> setting set.
>
> Signed-off-by: Niklas Söderlund 

Reviewed-by: Geert Uytterhoeven 

Will queue in sh-pfc-for-v4.11.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds


Re: [PATCH v3 03/13] drm: bridge: Link encoder and bridge in core code

2016-11-29 Thread Laurent Pinchart
Hi Daniel,

On Tuesday 29 Nov 2016 10:35:24 Daniel Vetter wrote:
> On Tue, Nov 29, 2016 at 11:04:33AM +0200, Laurent Pinchart wrote:
> > Instead of linking encoders and bridges in every driver (and getting it
> > wrong half of the time, as many drivers forget to set the drm_bridge
> > encoder pointer), do so in core code. The drm_bridge_attach() function
> > needs the encoder and optional previous bridge to perform that task,
> > update all the callers.
> > 
> > Signed-off-by: Laurent Pinchart
> > 
> > ---
> > 
> >  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c   |  4 +-
> >  drivers/gpu/drm/bridge/analogix/analogix_dp_core.c |  4 +-
> >  drivers/gpu/drm/bridge/dw-hdmi.c   |  3 +-
> >  drivers/gpu/drm/drm_bridge.c   | 46 +
> >  drivers/gpu/drm/drm_simple_kms_helper.c|  4 +-
> >  drivers/gpu/drm/exynos/exynos_dp.c |  5 +--
> >  drivers/gpu/drm/exynos/exynos_drm_dsi.c|  6 +--
> >  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c  |  5 +--
> >  drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c   |  5 +--
> >  drivers/gpu/drm/imx/imx-ldb.c  |  6 +--
> >  drivers/gpu/drm/imx/parallel-display.c |  4 +-
> >  drivers/gpu/drm/mediatek/mtk_dpi.c |  8 ++--
> >  drivers/gpu/drm/mediatek/mtk_dsi.c | 24 ++-
> >  drivers/gpu/drm/mediatek/mtk_hdmi.c| 11 +++---
> >  drivers/gpu/drm/msm/dsi/dsi_manager.c  | 17 +---
> >  drivers/gpu/drm/msm/edp/edp_bridge.c   |  2 +-
> >  drivers/gpu/drm/msm/hdmi/hdmi_bridge.c |  2 +-
> >  drivers/gpu/drm/rcar-du/rcar_du_hdmienc.c  |  5 +--
> >  drivers/gpu/drm/sti/sti_dvo.c  |  3 +-
> >  drivers/gpu/drm/sti/sti_hda.c  |  3 +-
> >  drivers/gpu/drm/sti/sti_hdmi.c |  3 +-
> >  drivers/gpu/drm/sun4i/sun4i_rgb.c  | 13 +++---
> >  include/drm/drm_bridge.h   |  3 +-
> >  23 files changed, 83 insertions(+), 103 deletions(-)

[snip]

> > diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
> > index 0ee052b7c21a..850bd6509ef1 100644
> > --- a/drivers/gpu/drm/drm_bridge.c
> > +++ b/drivers/gpu/drm/drm_bridge.c
> > @@ -26,6 +26,7 @@
> > 
> >  #include 
> >  #include 
> > +#include 
> > 
> >  /**
> >   * DOC: overview
> > @@ -92,32 +93,53 @@ void drm_bridge_remove(struct drm_bridge *bridge)
> >  EXPORT_SYMBOL(drm_bridge_remove);
> >  
> >  /**
> > - * drm_bridge_attach - associate given bridge to our DRM device
> > + * drm_bridge_attach - attach the bridge to an encoder's chain
> >   *
> > - * @dev: DRM device
> > - * @bridge: bridge control structure
> > + * @encoder: DRM encoder
> > + * @bridge: bridge to attach
> > + * @previous: previous bridge in the chain (optional)
> >   *
> > - * Called by a kms driver to link one of our encoder/bridge to the given
> > - * bridge.
> > + * Called by a kms driver to link the bridge to an encoder's chain. The
> > previous
> > + * argument specifies the previous bridge in the chain. If NULL, the
> > bridge is
> > + * linked directly at the encoder's output. Otherwise it is linked at the
> > + * previous bridge's output.
> >   *
> > - * Note that setting up links between the bridge and our encoder/bridge
> > - * objects needs to be handled by the kms driver itself.
> > + * If non-NULL the previous bridge must be already attached by a call to
> > this
> > + * function.
> >   *
> >   * RETURNS:
> >   * Zero on success, error code on failure
> >   */
> > -int drm_bridge_attach(struct drm_device *dev, struct drm_bridge *bridge)
> > +int drm_bridge_attach(struct drm_encoder *encoder, struct drm_bridge
> > *bridge,
> > + struct drm_bridge *previous)
> >  {
> > -   if (!dev || !bridge)
> > +   int ret;
> > +
> > +   if (!encoder || !bridge)
> > +   return -EINVAL;
> > +
> > +   if (previous && (!previous->dev || previous->encoder != encoder))
> > return -EINVAL;
> 
> Not sure we want to allow setting both encoder and bridge for chaining.
> I'd kinda expect that for chained use-case the bridge doesn't care that
> much who started the chain. And if not we can always create a helper to
> look up the drm_encoder.

As bridge drivers currently create connectors (I'd like to change that at some 
point, but one thing at a time) they need to call 
drm_mode_connector_attach_encoder() and thus need to have access to the 
drm_encoder object at the beginning of the chain. The drm_bridge structure has 
an encoder field, it seems to me that the easiest is to always populate it, 
regardless of the position of the bridge in the chain.

> Also, should we convert these to WARN_ON, they're kinda glaring driver (or
> well DT) bugs?

Yes, that seems like a good idea. I can add a patch on top of this one to 
convert the checks to WARN_ON, as I'd rather separate that change from the API 
change.

> Otherwise looks al

Re: [PATCH v3 01/13] drm: Don't include in

2016-11-29 Thread Laurent Pinchart
Hi Daniel,

On Tuesday 29 Nov 2016 10:30:36 Daniel Vetter wrote:
> On Tue, Nov 29, 2016 at 11:04:31AM +0200, Laurent Pinchart wrote:
> >  used to define most of the in-kernel KMS API. It has
> > now been split into separate files for each object type, but still
> > includes most other KMS headers to avoid breaking driver compilation.
> > 
> > As a step towards fixing that problem, remove the inclusion of
> >  from  and include it instead where
> > appropriate. Also remove the forward declarations of the drm_encoder and
> > drm_encoder_helper_funcs structures from  as they're not
> > needed in the header.
> > 
> >  now has to include  and contain a
> > forward declaration of struct drm_encoder in order to allow including it
> > as the first header in a compilation unit.
> > 
> > Signed-off-by: Laurent Pinchart
> > 
> 
> Assuming it all still compiles the various defconfigs we have (I didn't
> check that, but lgtm otherwise):

I've done my best to try and compile all affected drivers. 0day hasn't 
complained either so far, so we should be good.

> Reviewed-by: Daniel Vetter 
> 
> > ---
> > 
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h| 1 +
> >  drivers/gpu/drm/ast/ast_drv.h   | 1 +
> >  drivers/gpu/drm/bochs/bochs.h   | 1 +
> >  drivers/gpu/drm/cirrus/cirrus_drv.h | 1 +
> >  drivers/gpu/drm/drm_connector.c | 1 +
> >  drivers/gpu/drm/drm_crtc_helper.c   | 1 +
> >  drivers/gpu/drm/drm_edid.c  | 1 +
> >  drivers/gpu/drm/drm_mode_config.c   | 1 +
> >  drivers/gpu/drm/drm_of.c| 1 +
> >  drivers/gpu/drm/drm_plane_helper.c  | 1 +
> >  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h   | 2 ++
> >  drivers/gpu/drm/gma500/psb_intel_drv.h  | 1 +
> >  drivers/gpu/drm/i915/intel_drv.h| 1 +
> >  drivers/gpu/drm/mgag200/mgag200_drv.h   | 1 +
> >  drivers/gpu/drm/nouveau/nouveau_connector.h | 1 +
> >  drivers/gpu/drm/qxl/qxl_drv.h   | 1 +
> >  drivers/gpu/drm/radeon/radeon_mode.h| 1 +
> >  drivers/gpu/drm/rcar-du/rcar_du_encoder.h   | 1 +
> >  drivers/gpu/drm/shmobile/shmob_drm_crtc.h   | 1 +
> >  drivers/gpu/drm/tegra/drm.h | 1 +
> >  drivers/gpu/drm/vc4/vc4_drv.h   | 2 ++
> >  drivers/gpu/drm/virtio/virtgpu_drv.h| 1 +
> >  drivers/gpu/drm/vmwgfx/vmwgfx_kms.h | 1 +
> >  include/drm/drm_crtc.h  | 3 ---
> >  include/drm/drm_encoder.h   | 3 +++
> >  include/drm/drm_encoder_slave.h | 1 +
> >  include/drm/drm_modeset_helper_vtables.h| 1 +
> >  27 files changed, 30 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h index
> > 1e23334b07fb..fac06064a8f5 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
> > @@ -32,6 +32,7 @@
> > 
> >  #include 
> >  #include 
> > 
> > +#include 
> > 
> >  #include 
> >  #include 
> >  #include 
> > 
> > diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
> > index 908011d2c8f5..6f3b6f50cf52 100644
> > --- a/drivers/gpu/drm/ast/ast_drv.h
> > +++ b/drivers/gpu/drm/ast/ast_drv.h
> > @@ -28,6 +28,7 @@
> > 
> >  #ifndef __AST_DRV_H__
> >  #define __AST_DRV_H__
> > 
> > +#include 
> > 
> >  #include 
> >  
> >  #include 
> > 
> > diff --git a/drivers/gpu/drm/bochs/bochs.h b/drivers/gpu/drm/bochs/bochs.h
> > index 32dfe418cc98..f626bab7f5e3 100644
> > --- a/drivers/gpu/drm/bochs/bochs.h
> > +++ b/drivers/gpu/drm/bochs/bochs.h
> > @@ -4,6 +4,7 @@
> > 
> >  #include 
> >  #include 
> >  #include 
> > 
> > +#include 
> > 
> >  #include 
> >  
> >  #include 
> > 
> > diff --git a/drivers/gpu/drm/cirrus/cirrus_drv.h
> > b/drivers/gpu/drm/cirrus/cirrus_drv.h index 2188d6b61b3e..b59aeef4635a
> > 100644
> > --- a/drivers/gpu/drm/cirrus/cirrus_drv.h
> > +++ b/drivers/gpu/drm/cirrus/cirrus_drv.h
> > @@ -13,6 +13,7 @@
> > 
> >  #include 
> > 
> > +#include 
> > 
> >  #include 
> >  
> >  #include 
> > 
> > diff --git a/drivers/gpu/drm/drm_connector.c
> > b/drivers/gpu/drm/drm_connector.c index b5c6a8ee831e..5f1e1f190d30 100644
> > --- a/drivers/gpu/drm/drm_connector.c
> > +++ b/drivers/gpu/drm/drm_connector.c
> > @@ -23,6 +23,7 @@
> > 
> >  #include 
> >  #include 
> >  #include 
> > 
> > +#include 
> > 
> >  #include "drm_crtc_internal.h"
> >  #include "drm_internal.h"
> > 
> > diff --git a/drivers/gpu/drm/drm_crtc_helper.c
> > b/drivers/gpu/drm/drm_crtc_helper.c index 5d2cb138eba6..b3fc23313cc3
> > 100644
> > --- a/drivers/gpu/drm/drm_crtc_helper.c
> > +++ b/drivers/gpu/drm/drm_crtc_helper.c
> > @@ -36,6 +36,7 @@
> > 
> >  #include 
> >  #include 
> >  #include 
> > 
> > +#include 
> > 
> >  #include 
> >  #include 
> >  #include 
> > 
> > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> > index 7eec18925b70..a9e3cc3990c1 100644
> > --- a/drivers/gpu/drm/drm_edid.c
> > +++ b/drivers/gpu/drm/

Re: [PATCH v3 03/13] drm: bridge: Link encoder and bridge in core code

2016-11-29 Thread Daniel Vetter
On Tue, Nov 29, 2016 at 11:04:33AM +0200, Laurent Pinchart wrote:
> Instead of linking encoders and bridges in every driver (and getting it
> wrong half of the time, as many drivers forget to set the drm_bridge
> encoder pointer), do so in core code. The drm_bridge_attach() function
> needs the encoder and optional previous bridge to perform that task,
> update all the callers.
> 
> Signed-off-by: Laurent Pinchart 
> ---
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c   |  4 +-
>  drivers/gpu/drm/bridge/analogix/analogix_dp_core.c |  4 +-
>  drivers/gpu/drm/bridge/dw-hdmi.c   |  3 +-
>  drivers/gpu/drm/drm_bridge.c   | 46 
> --
>  drivers/gpu/drm/drm_simple_kms_helper.c|  4 +-
>  drivers/gpu/drm/exynos/exynos_dp.c |  5 +--
>  drivers/gpu/drm/exynos/exynos_drm_dsi.c|  6 +--
>  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c  |  5 +--
>  drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c   |  5 +--
>  drivers/gpu/drm/imx/imx-ldb.c  |  6 +--
>  drivers/gpu/drm/imx/parallel-display.c |  4 +-
>  drivers/gpu/drm/mediatek/mtk_dpi.c |  8 ++--
>  drivers/gpu/drm/mediatek/mtk_dsi.c | 24 ++-
>  drivers/gpu/drm/mediatek/mtk_hdmi.c| 11 +++---
>  drivers/gpu/drm/msm/dsi/dsi_manager.c  | 17 +---
>  drivers/gpu/drm/msm/edp/edp_bridge.c   |  2 +-
>  drivers/gpu/drm/msm/hdmi/hdmi_bridge.c |  2 +-
>  drivers/gpu/drm/rcar-du/rcar_du_hdmienc.c  |  5 +--
>  drivers/gpu/drm/sti/sti_dvo.c  |  3 +-
>  drivers/gpu/drm/sti/sti_hda.c  |  3 +-
>  drivers/gpu/drm/sti/sti_hdmi.c |  3 +-
>  drivers/gpu/drm/sun4i/sun4i_rgb.c  | 13 +++---
>  include/drm/drm_bridge.h   |  3 +-
>  23 files changed, 83 insertions(+), 103 deletions(-)
> 
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c 
> b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c
> index 6119b5085501..e7799b6ee829 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c
> @@ -230,9 +230,7 @@ static int atmel_hlcdc_attach_endpoint(struct drm_device 
> *dev,
>   of_node_put(np);
>  
>   if (bridge) {
> - output->encoder.bridge = bridge;
> - bridge->encoder = &output->encoder;
> - ret = drm_bridge_attach(dev, bridge);
> + ret = drm_bridge_attach(&output->encoder, bridge, NULL);
>   if (!ret)
>   return 0;
>   }
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c 
> b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> index 6e0447f329a2..1835f1fdad19 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> @@ -1227,12 +1227,10 @@ static int analogix_dp_create_bridge(struct 
> drm_device *drm_dev,
>  
>   dp->bridge = bridge;
>  
> - dp->encoder->bridge = bridge;
>   bridge->driver_private = dp;
> - bridge->encoder = dp->encoder;
>   bridge->funcs = &analogix_dp_bridge_funcs;
>  
> - ret = drm_bridge_attach(drm_dev, bridge);
> + ret = drm_bridge_attach(dp->encoder, bridge, NULL);
>   if (ret) {
>   DRM_ERROR("failed to attach drm bridge\n");
>   return -EINVAL;
> diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c 
> b/drivers/gpu/drm/bridge/dw-hdmi.c
> index b71088dab268..432e0e3fff72 100644
> --- a/drivers/gpu/drm/bridge/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/dw-hdmi.c
> @@ -1841,13 +1841,12 @@ static int dw_hdmi_register(struct drm_device *drm, 
> struct dw_hdmi *hdmi)
>   hdmi->bridge = bridge;
>   bridge->driver_private = hdmi;
>   bridge->funcs = &dw_hdmi_bridge_funcs;
> - ret = drm_bridge_attach(drm, bridge);
> + ret = drm_bridge_attach(encoder, bridge, NULL);
>   if (ret) {
>   DRM_ERROR("Failed to initialize bridge with drm\n");
>   return -EINVAL;
>   }
>  
> - encoder->bridge = bridge;
>   hdmi->connector.polled = DRM_CONNECTOR_POLL_HPD;
>  
>   drm_connector_helper_add(&hdmi->connector,
> diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
> index 0ee052b7c21a..850bd6509ef1 100644
> --- a/drivers/gpu/drm/drm_bridge.c
> +++ b/drivers/gpu/drm/drm_bridge.c
> @@ -26,6 +26,7 @@
>  #include 
>  
>  #include 
> +#include 
>  
>  /**
>   * DOC: overview
> @@ -92,32 +93,53 @@ void drm_bridge_remove(struct drm_bridge *bridge)
>  EXPORT_SYMBOL(drm_bridge_remove);
>  
>  /**
> - * drm_bridge_attach - associate given bridge to our DRM device
> + * drm_bridge_attach - attach the bridge to an encoder's chain
>   *
> - * @dev: DRM device
> - * @bridge: bridge control structure
> + * @encoder: DRM encoder
> + * @bridge: bridge to attach
> + * @previous: previous bridge in the ch

Re: [PATCH v3 02/13] drm: Fix compilation warning caused by static inline forward declaration

2016-11-29 Thread Daniel Vetter
On Tue, Nov 29, 2016 at 11:04:32AM +0200, Laurent Pinchart wrote:
> The drm_crtc_mask() function used in  is a static
> inline defined in . If the first header is included in a
> compilation unit without the second one, the following compilation
> warning will be issued.
> 
> In file included from /drivers/gpu/drm/drm_bridge.c:29:0:
> /include/drm/drm_encoder.h:192:95: warning: ‘drm_crtc_mask’ used but 
> never defined
>  static inline uint32_t drm_crtc_mask(const struct drm_crtc *crtc);
> 
> Fix this by including the header defining the function instead of using
> a forward declaration.
> 
> Signed-off-by: Laurent Pinchart 

Reviewed-by: Daniel Vetter 

> ---
>  include/drm/drm_encoder.h | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/include/drm/drm_encoder.h b/include/drm/drm_encoder.h
> index cec6ac45c6cc..5f58f65344e0 100644
> --- a/include/drm/drm_encoder.h
> +++ b/include/drm/drm_encoder.h
> @@ -25,6 +25,7 @@
>  
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  
> @@ -191,9 +192,6 @@ static inline unsigned int drm_encoder_index(struct 
> drm_encoder *encoder)
>   return encoder->index;
>  }
>  
> -/* FIXME: We have an include file mess still, drm_crtc.h needs untangling. */
> -static inline uint32_t drm_crtc_mask(const struct drm_crtc *crtc);
> -
>  /**
>   * drm_encoder_crtc_ok - can a given crtc drive a given encoder?
>   * @encoder: encoder to test
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch


Re: [PATCH v3 01/13] drm: Don't include in

2016-11-29 Thread Daniel Vetter
On Tue, Nov 29, 2016 at 11:04:31AM +0200, Laurent Pinchart wrote:
>  used to define most of the in-kernel KMS API. It has
> now been split into separate files for each object type, but still
> includes most other KMS headers to avoid breaking driver compilation.
> 
> As a step towards fixing that problem, remove the inclusion of
>  from  and include it instead where
> appropriate. Also remove the forward declarations of the drm_encoder and
> drm_encoder_helper_funcs structures from  as they're not
> needed in the header.
> 
>  now has to include  and contain a
> forward declaration of struct drm_encoder in order to allow including it
> as the first header in a compilation unit.
> 
> Signed-off-by: Laurent Pinchart 

Assuming it all still compiles the various defconfigs we have (I didn't
check that, but lgtm otherwise):

Reviewed-by: Daniel Vetter 
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h| 1 +
>  drivers/gpu/drm/ast/ast_drv.h   | 1 +
>  drivers/gpu/drm/bochs/bochs.h   | 1 +
>  drivers/gpu/drm/cirrus/cirrus_drv.h | 1 +
>  drivers/gpu/drm/drm_connector.c | 1 +
>  drivers/gpu/drm/drm_crtc_helper.c   | 1 +
>  drivers/gpu/drm/drm_edid.c  | 1 +
>  drivers/gpu/drm/drm_mode_config.c   | 1 +
>  drivers/gpu/drm/drm_of.c| 1 +
>  drivers/gpu/drm/drm_plane_helper.c  | 1 +
>  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h   | 2 ++
>  drivers/gpu/drm/gma500/psb_intel_drv.h  | 1 +
>  drivers/gpu/drm/i915/intel_drv.h| 1 +
>  drivers/gpu/drm/mgag200/mgag200_drv.h   | 1 +
>  drivers/gpu/drm/nouveau/nouveau_connector.h | 1 +
>  drivers/gpu/drm/qxl/qxl_drv.h   | 1 +
>  drivers/gpu/drm/radeon/radeon_mode.h| 1 +
>  drivers/gpu/drm/rcar-du/rcar_du_encoder.h   | 1 +
>  drivers/gpu/drm/shmobile/shmob_drm_crtc.h   | 1 +
>  drivers/gpu/drm/tegra/drm.h | 1 +
>  drivers/gpu/drm/vc4/vc4_drv.h   | 2 ++
>  drivers/gpu/drm/virtio/virtgpu_drv.h| 1 +
>  drivers/gpu/drm/vmwgfx/vmwgfx_kms.h | 1 +
>  include/drm/drm_crtc.h  | 3 ---
>  include/drm/drm_encoder.h   | 3 +++
>  include/drm/drm_encoder_slave.h | 1 +
>  include/drm/drm_modeset_helper_vtables.h| 1 +
>  27 files changed, 30 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
> index 1e23334b07fb..fac06064a8f5 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
> @@ -32,6 +32,7 @@
>  
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
> index 908011d2c8f5..6f3b6f50cf52 100644
> --- a/drivers/gpu/drm/ast/ast_drv.h
> +++ b/drivers/gpu/drm/ast/ast_drv.h
> @@ -28,6 +28,7 @@
>  #ifndef __AST_DRV_H__
>  #define __AST_DRV_H__
>  
> +#include 
>  #include 
>  
>  #include 
> diff --git a/drivers/gpu/drm/bochs/bochs.h b/drivers/gpu/drm/bochs/bochs.h
> index 32dfe418cc98..f626bab7f5e3 100644
> --- a/drivers/gpu/drm/bochs/bochs.h
> +++ b/drivers/gpu/drm/bochs/bochs.h
> @@ -4,6 +4,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  
>  #include 
> diff --git a/drivers/gpu/drm/cirrus/cirrus_drv.h 
> b/drivers/gpu/drm/cirrus/cirrus_drv.h
> index 2188d6b61b3e..b59aeef4635a 100644
> --- a/drivers/gpu/drm/cirrus/cirrus_drv.h
> +++ b/drivers/gpu/drm/cirrus/cirrus_drv.h
> @@ -13,6 +13,7 @@
>  
>  #include 
>  
> +#include 
>  #include 
>  
>  #include 
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index b5c6a8ee831e..5f1e1f190d30 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -23,6 +23,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include "drm_crtc_internal.h"
>  #include "drm_internal.h"
> diff --git a/drivers/gpu/drm/drm_crtc_helper.c 
> b/drivers/gpu/drm/drm_crtc_helper.c
> index 5d2cb138eba6..b3fc23313cc3 100644
> --- a/drivers/gpu/drm/drm_crtc_helper.c
> +++ b/drivers/gpu/drm/drm_crtc_helper.c
> @@ -36,6 +36,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 7eec18925b70..a9e3cc3990c1 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -35,6 +35,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  
>  #define version_greater(edid, maj, min) \
> diff --git a/drivers/gpu/drm/drm_mode_config.c 
> b/drivers/gpu/drm/drm_mode_config.c
> index 2735a5847ffa..09b1d8f267a6 100644
> --- a/drivers/gpu/drm/drm_mode_config.c
> +++ b/drivers/gpu/drm/drm_mode_config.c
> @@ -20,6 +20,7 @@
>   * OF THIS SOFTWARE.
>   */
>  
> +#include 
>  #include 
>  #include 
>  
> diff --git a/drivers/gpu/drm/drm_of.c b/drivers/gpu/drm/drm_of.c
> index 47848ed8ca4

[PATCH v3 12/13] drm: rcar-du: Replace manual bridge implementation with DRM bridge

2016-11-29 Thread Laurent Pinchart
The rcar-du driver contains a manual implementation of HDMI and VGA
bridges. Use DRM bridges to replace it.

Signed-off-by: Laurent Pinchart 
---
 drivers/gpu/drm/rcar-du/Kconfig   |   6 --
 drivers/gpu/drm/rcar-du/Makefile  |   5 +-
 drivers/gpu/drm/rcar-du/rcar_du_encoder.c | 104 +--
 drivers/gpu/drm/rcar-du/rcar_du_encoder.h |   2 -
 drivers/gpu/drm/rcar-du/rcar_du_hdmienc.c | 134 --
 drivers/gpu/drm/rcar-du/rcar_du_hdmienc.h |  35 
 drivers/gpu/drm/rcar-du/rcar_du_vgacon.c  |  82 --
 drivers/gpu/drm/rcar-du/rcar_du_vgacon.h  |  23 -
 8 files changed, 60 insertions(+), 331 deletions(-)
 delete mode 100644 drivers/gpu/drm/rcar-du/rcar_du_hdmienc.c
 delete mode 100644 drivers/gpu/drm/rcar-du/rcar_du_hdmienc.h
 delete mode 100644 drivers/gpu/drm/rcar-du/rcar_du_vgacon.c
 delete mode 100644 drivers/gpu/drm/rcar-du/rcar_du_vgacon.h

diff --git a/drivers/gpu/drm/rcar-du/Kconfig b/drivers/gpu/drm/rcar-du/Kconfig
index 2bab449add76..06121eeba9e5 100644
--- a/drivers/gpu/drm/rcar-du/Kconfig
+++ b/drivers/gpu/drm/rcar-du/Kconfig
@@ -11,12 +11,6 @@ config DRM_RCAR_DU
  Choose this option if you have an R-Car chipset.
  If M is selected the module will be called rcar-du-drm.
 
-config DRM_RCAR_HDMI
-   bool "R-Car DU HDMI Encoder Support"
-   depends on DRM_RCAR_DU
-   help
- Enable support for external HDMI encoders.
-
 config DRM_RCAR_LVDS
bool "R-Car DU LVDS Encoder Support"
depends on DRM_RCAR_DU
diff --git a/drivers/gpu/drm/rcar-du/Makefile b/drivers/gpu/drm/rcar-du/Makefile
index d3b44651061a..a492e6858691 100644
--- a/drivers/gpu/drm/rcar-du/Makefile
+++ b/drivers/gpu/drm/rcar-du/Makefile
@@ -4,10 +4,7 @@ rcar-du-drm-y := rcar_du_crtc.o \
 rcar_du_group.o \
 rcar_du_kms.o \
 rcar_du_lvdscon.o \
-rcar_du_plane.o \
-rcar_du_vgacon.o
-
-rcar-du-drm-$(CONFIG_DRM_RCAR_HDMI)+= rcar_du_hdmienc.o
+rcar_du_plane.o
 
 rcar-du-drm-$(CONFIG_DRM_RCAR_LVDS)+= rcar_du_lvdsenc.o
 
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_encoder.c 
b/drivers/gpu/drm/rcar-du/rcar_du_encoder.c
index 21262057ef08..d412dc622bfa 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_encoder.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_encoder.c
@@ -20,11 +20,9 @@
 
 #include "rcar_du_drv.h"
 #include "rcar_du_encoder.h"
-#include "rcar_du_hdmienc.h"
 #include "rcar_du_kms.h"
 #include "rcar_du_lvdscon.h"
 #include "rcar_du_lvdsenc.h"
-#include "rcar_du_vgacon.h"
 
 /* 
-
  * Encoder
@@ -63,29 +61,35 @@ static int rcar_du_encoder_atomic_check(struct drm_encoder 
*encoder,
struct rcar_du_encoder *renc = to_rcar_encoder(encoder);
struct drm_display_mode *adjusted_mode = &crtc_state->adjusted_mode;
const struct drm_display_mode *mode = &crtc_state->mode;
-   const struct drm_display_mode *panel_mode;
struct drm_connector *connector = conn_state->connector;
struct drm_device *dev = encoder->dev;
 
-   /* DAC encoders have currently no restriction on the mode. */
-   if (encoder->encoder_type == DRM_MODE_ENCODER_DAC)
-   return 0;
+   /*
+* Only panel-related encoder types require validation here, everything
+* else is handled by the bridge drivers.
+*/
+   if (encoder->encoder_type == DRM_MODE_ENCODER_LVDS) {
+   const struct drm_display_mode *panel_mode;
 
-   if (list_empty(&connector->modes)) {
-   dev_dbg(dev->dev, "encoder: empty modes list\n");
-   return -EINVAL;
-   }
+   if (list_empty(&connector->modes)) {
+   dev_dbg(dev->dev, "encoder: empty modes list\n");
+   return -EINVAL;
+   }
 
-   panel_mode = list_first_entry(&connector->modes,
- struct drm_display_mode, head);
+   panel_mode = list_first_entry(&connector->modes,
+ struct drm_display_mode, head);
 
-   /* We're not allowed to modify the resolution. */
-   if (mode->hdisplay != panel_mode->hdisplay ||
-   mode->vdisplay != panel_mode->vdisplay)
-   return -EINVAL;
+   /* We're not allowed to modify the resolution. */
+   if (mode->hdisplay != panel_mode->hdisplay ||
+   mode->vdisplay != panel_mode->vdisplay)
+   return -EINVAL;
 
-   /* The flat panel mode is fixed, just copy it to the adjusted mode. */
-   drm_mode_copy(adjusted_mode, panel_mode);
+   /*
+* The flat panel mode is fixed, just copy it to the adjusted
+* mode.
+*/
+   drm_mode_copy(adjusted_mode, panel_mode);
+   }
 
if (renc->lvds)

[PATCH v3 09/13] drm: Add encoder_type field to the drm_bridge structure

2016-11-29 Thread Laurent Pinchart
The drm_bridge object models on- or off-chip hardware encoders and
provide an abstract control API to display drivers. In order to help
display drivers creating the right kind of drm_encoder object, expose
the type of the hardware encoder associated with each bridge.

Signed-off-by: Laurent Pinchart 
---
Changes since v1:

- Clarify documentation
---
 include/drm/drm_bridge.h | 8 
 1 file changed, 8 insertions(+)

diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
index 94e5ee96b3b5..0643b6ce27de 100644
--- a/include/drm/drm_bridge.h
+++ b/include/drm/drm_bridge.h
@@ -194,6 +194,14 @@ struct drm_bridge {
 #endif
struct list_head list;
 
+   /**
+* @encoder_type:
+*
+* Type of the hardware encoder modeled by the bridge as one of the
+* DRM_MODE_ENCODER_* types. For the last bridge in the chain this will
+* usually be identical to the bridge's &drm_encoder encoder_type.
+*/
+   int encoder_type;
const struct drm_bridge_funcs *funcs;
void *driver_private;
 };
-- 
Regards,

Laurent Pinchart



[PATCH v3 13/13] drm: rcar-du: Initialize encoder's type based on the bridge's type

2016-11-29 Thread Laurent Pinchart
Set the type of the DRM encoder that models the hardware encoders
(bridges) chain based on the type of the last bridge in the chain
instead of hardcoding encoder compatible strings in the display driver.

Signed-off-by: Laurent Pinchart 
---
 drivers/gpu/drm/rcar-du/rcar_du_encoder.c | 17 ++
 drivers/gpu/drm/rcar-du/rcar_du_encoder.h |  1 -
 drivers/gpu/drm/rcar-du/rcar_du_kms.c | 37 ++-
 3 files changed, 4 insertions(+), 51 deletions(-)

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_encoder.c 
b/drivers/gpu/drm/rcar-du/rcar_du_encoder.c
index d412dc622bfa..1adf4a77904e 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_encoder.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_encoder.c
@@ -156,7 +156,6 @@ static const struct drm_encoder_funcs encoder_funcs = {
 };
 
 int rcar_du_encoder_init(struct rcar_du_device *rcdu,
-enum rcar_du_encoder_type type,
 enum rcar_du_output output,
 struct device_node *enc_node,
 struct device_node *con_node)
@@ -194,23 +193,11 @@ int rcar_du_encoder_init(struct rcar_du_device *rcdu,
ret = -EPROBE_DEFER;
goto done;
}
-   }
 
-   switch (type) {
-   case RCAR_DU_ENCODER_VGA:
-   encoder_type = DRM_MODE_ENCODER_DAC;
-   break;
-   case RCAR_DU_ENCODER_LVDS:
-   encoder_type = DRM_MODE_ENCODER_LVDS;
-   break;
-   case RCAR_DU_ENCODER_HDMI:
-   encoder_type = DRM_MODE_ENCODER_TMDS;
-   break;
-   case RCAR_DU_ENCODER_NONE:
-   default:
+   encoder_type = bridge->encoder_type;
+   } else {
/* No external encoder, use the internal encoder type. */
encoder_type = rcdu->info->routes[output].encoder_type;
-   break;
}
 
ret = drm_encoder_init(rcdu->ddev, encoder, &encoder_funcs,
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_encoder.h 
b/drivers/gpu/drm/rcar-du/rcar_du_encoder.h
index c1cfbe0d54ce..2b9036e7f6fe 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_encoder.h
+++ b/drivers/gpu/drm/rcar-du/rcar_du_encoder.h
@@ -51,7 +51,6 @@ struct rcar_du_connector {
container_of(c, struct rcar_du_connector, connector)
 
 int rcar_du_encoder_init(struct rcar_du_device *rcdu,
-enum rcar_du_encoder_type type,
 enum rcar_du_output output,
 struct device_node *enc_node,
 struct device_node *con_node);
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c 
b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
index b5d3f16cfa12..aa84689043ae 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
@@ -366,16 +366,6 @@ static int rcar_du_encoders_init_one(struct rcar_du_device 
*rcdu,
 enum rcar_du_output output,
 struct of_endpoint *ep)
 {
-   static const struct {
-   const char *compatible;
-   enum rcar_du_encoder_type type;
-   } encoders[] = {
-   { "adi,adv7123", RCAR_DU_ENCODER_VGA },
-   { "adi,adv7511w", RCAR_DU_ENCODER_HDMI },
-   { "thine,thc63lvdm83d", RCAR_DU_ENCODER_LVDS },
-   };
-
-   enum rcar_du_encoder_type enc_type = RCAR_DU_ENCODER_NONE;
struct device_node *connector = NULL;
struct device_node *encoder = NULL;
struct device_node *ep_node = NULL;
@@ -422,30 +412,7 @@ static int rcar_du_encoders_init_one(struct rcar_du_device 
*rcdu,
 
of_node_put(entity_ep_node);
 
-   if (encoder) {
-   /*
-* If an encoder has been found, get its type based on its
-* compatible string.
-*/
-   unsigned int i;
-
-   for (i = 0; i < ARRAY_SIZE(encoders); ++i) {
-   if (of_device_is_compatible(encoder,
-   encoders[i].compatible)) {
-   enc_type = encoders[i].type;
-   break;
-   }
-   }
-
-   if (i == ARRAY_SIZE(encoders)) {
-   dev_warn(rcdu->dev,
-"unknown encoder type for %s, skipping\n",
-encoder->full_name);
-   of_node_put(encoder);
-   of_node_put(connector);
-   return -EINVAL;
-   }
-   } else {
+   if (!encoder) {
/*
 * If no encoder has been found the entity must be the
 * connector.
@@ -453,7 +420,7 @@ static int rcar_du_encoders_init_one(struct rcar_du_device 
*rcdu,
connector = entity;
}
 
-   ret = rcar_du_encoder_init(rcdu, enc_type, outpu

[PATCH v3 10/13] drm: bridge: Set bridges' encoder type

2016-11-29 Thread Laurent Pinchart
Initialize the new drm_bridge::encoder_type field to the right value for
all external bridge drivers.

Signed-off-by: Laurent Pinchart 
---
 drivers/gpu/drm/bridge/adv7511/adv7511_drv.c   | 1 +
 drivers/gpu/drm/bridge/analogix-anx78xx.c  | 1 +
 drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 1 +
 drivers/gpu/drm/bridge/dumb-vga-dac.c  | 1 +
 drivers/gpu/drm/bridge/dw-hdmi.c   | 2 ++
 drivers/gpu/drm/bridge/nxp-ptn3460.c   | 2 ++
 drivers/gpu/drm/bridge/parade-ps8622.c | 2 ++
 drivers/gpu/drm/bridge/sii902x.c   | 2 ++
 drivers/gpu/drm/bridge/tc358767.c  | 2 ++
 9 files changed, 14 insertions(+)

diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c 
b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
index 8ed3906dd411..66bbd2073529 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
@@ -1030,6 +1030,7 @@ static int adv7511_probe(struct i2c_client *i2c, const 
struct i2c_device_id *id)
 
adv7511->bridge.funcs = &adv7511_bridge_funcs;
adv7511->bridge.of_node = dev->of_node;
+   adv7511->bridge.encoder_type = DRM_MODE_ENCODER_TMDS;
 
ret = drm_bridge_add(&adv7511->bridge);
if (ret) {
diff --git a/drivers/gpu/drm/bridge/analogix-anx78xx.c 
b/drivers/gpu/drm/bridge/analogix-anx78xx.c
index a2a82366a771..62be93f7ffaa 100644
--- a/drivers/gpu/drm/bridge/analogix-anx78xx.c
+++ b/drivers/gpu/drm/bridge/analogix-anx78xx.c
@@ -1437,6 +1437,7 @@ static int anx78xx_i2c_probe(struct i2c_client *client,
}
 
anx78xx->bridge.funcs = &anx78xx_bridge_funcs;
+   anx78xx->bridge.encoder_type = DRM_MODE_ENCODER_TMDS;
 
err = drm_bridge_add(&anx78xx->bridge);
if (err < 0) {
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c 
b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
index 1835f1fdad19..f0f9c5024e3e 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
@@ -1229,6 +1229,7 @@ static int analogix_dp_create_bridge(struct drm_device 
*drm_dev,
 
bridge->driver_private = dp;
bridge->funcs = &analogix_dp_bridge_funcs;
+   bridge->encoder_type = DRM_MODE_ENCODER_TMDS;
 
ret = drm_bridge_attach(dp->encoder, bridge, NULL);
if (ret) {
diff --git a/drivers/gpu/drm/bridge/dumb-vga-dac.c 
b/drivers/gpu/drm/bridge/dumb-vga-dac.c
index b33e3f829e4f..036c54c849e6 100644
--- a/drivers/gpu/drm/bridge/dumb-vga-dac.c
+++ b/drivers/gpu/drm/bridge/dumb-vga-dac.c
@@ -182,6 +182,7 @@ static int dumb_vga_probe(struct platform_device *pdev)
 
vga->bridge.funcs = &dumb_vga_bridge_funcs;
vga->bridge.of_node = pdev->dev.of_node;
+   vga->bridge.encoder_type = DRM_MODE_ENCODER_DAC;
 
ret = drm_bridge_add(&vga->bridge);
if (ret && !IS_ERR(vga->ddc))
diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c b/drivers/gpu/drm/bridge/dw-hdmi.c
index 432e0e3fff72..0ac28aa76017 100644
--- a/drivers/gpu/drm/bridge/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/dw-hdmi.c
@@ -1841,6 +1841,8 @@ static int dw_hdmi_register(struct drm_device *drm, 
struct dw_hdmi *hdmi)
hdmi->bridge = bridge;
bridge->driver_private = hdmi;
bridge->funcs = &dw_hdmi_bridge_funcs;
+   bridge->encoder_type = DRM_MODE_ENCODER_TMDS;
+
ret = drm_bridge_attach(encoder, bridge, NULL);
if (ret) {
DRM_ERROR("Failed to initialize bridge with drm\n");
diff --git a/drivers/gpu/drm/bridge/nxp-ptn3460.c 
b/drivers/gpu/drm/bridge/nxp-ptn3460.c
index f1a99938e924..8be117cc024f 100644
--- a/drivers/gpu/drm/bridge/nxp-ptn3460.c
+++ b/drivers/gpu/drm/bridge/nxp-ptn3460.c
@@ -349,6 +349,8 @@ static int ptn3460_probe(struct i2c_client *client,
 
ptn_bridge->bridge.funcs = &ptn3460_bridge_funcs;
ptn_bridge->bridge.of_node = dev->of_node;
+   ptn_bridge->bridge.encoder_type = DRM_MODE_ENCODER_LVDS;
+
ret = drm_bridge_add(&ptn_bridge->bridge);
if (ret) {
DRM_ERROR("Failed to add bridge\n");
diff --git a/drivers/gpu/drm/bridge/parade-ps8622.c 
b/drivers/gpu/drm/bridge/parade-ps8622.c
index 6f7c2f9860d2..37ac7211139a 100644
--- a/drivers/gpu/drm/bridge/parade-ps8622.c
+++ b/drivers/gpu/drm/bridge/parade-ps8622.c
@@ -615,6 +615,8 @@ static int ps8622_probe(struct i2c_client *client,
 
ps8622->bridge.funcs = &ps8622_bridge_funcs;
ps8622->bridge.of_node = dev->of_node;
+   ps8622->bridge.encoder_type = DRM_MODE_ENCODER_LVDS;
+
ret = drm_bridge_add(&ps8622->bridge);
if (ret) {
DRM_ERROR("Failed to add bridge\n");
diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c
index 9126d0306ab5..ad4663dea333 100644
--- a/drivers/gpu/drm/bridge/sii902x.c
+++ b/drivers/gpu/drm/bridge/sii902x.c
@@ -418,6 +418,8 @@ static int sii902x_probe(struct i2c_client *client,
 
sii90

[PATCH v3 04/13] drm: bridge: Detach bridge from encoder at encoder cleanup time

2016-11-29 Thread Laurent Pinchart
Most drivers that use bridges forgot to detach them at cleanup time.
Instead of fixing them one by one, detach the bridge in the core
drm_encoder_cleanup() function.

Signed-off-by: Laurent Pinchart 
---
 drivers/gpu/drm/drm_encoder.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/drm_encoder.c b/drivers/gpu/drm/drm_encoder.c
index 5c067719164d..9c1f99646e0d 100644
--- a/drivers/gpu/drm/drm_encoder.c
+++ b/drivers/gpu/drm/drm_encoder.c
@@ -164,6 +164,9 @@ void drm_encoder_cleanup(struct drm_encoder *encoder)
 * the indices on the drm_encoder after us in the encoder_list.
 */
 
+   if (encoder->bridge)
+   drm_bridge_detach(encoder->bridge);
+
drm_modeset_lock_all(dev);
drm_mode_object_unregister(dev, &encoder->base);
kfree(encoder->name);
-- 
Regards,

Laurent Pinchart



[PATCH v3 08/13] drm: bridge: lvds-encoder: Add thine,thc63lvdm83d compatible string

2016-11-29 Thread Laurent Pinchart
The THC63LVDM83D is a transparent LVDS encoder. Unlike dumb LVDS
encoders it can be controlled through a few pins (power down, LVDS
swing, clock edge selection) and requires power supplies. However, on
several boards where the device is used neither the control pins nor the
power supply are controllable.

To avoid developing a separate device-specific driver add a
"thine,thc63lvdm83d" compatible entry to the lvds-encoder driver. This
will allow supporting many THC63LVDM83D-based boards easily, while
allowing future development of an thc63lvdm83d driver when needed
without breaking backward compatibility.

Signed-off-by: Laurent Pinchart 
---
 drivers/gpu/drm/bridge/lvds-encoder.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/bridge/lvds-encoder.c 
b/drivers/gpu/drm/bridge/lvds-encoder.c
index 576770ea8d7e..16c5ac77824a 100644
--- a/drivers/gpu/drm/bridge/lvds-encoder.c
+++ b/drivers/gpu/drm/bridge/lvds-encoder.c
@@ -198,6 +198,7 @@ static int lvds_encoder_remove(struct platform_device *pdev)
 
 static const struct of_device_id lvds_encoder_match[] = {
{ .compatible = "lvds-encoder" },
+   { .compatible = "thine,thc63lvdm83d" },
{},
 };
 MODULE_DEVICE_TABLE(of, lvds_encoder_match);
-- 
Regards,

Laurent Pinchart



[PATCH v3 11/13] drm: Set on-chip bridges' encoder type

2016-11-29 Thread Laurent Pinchart
Initialize the new drm_bridge::encoder_type field to the right value for
all bridges that model on-SoC IP cores.

Signed-off-by: Laurent Pinchart 
---
 drivers/gpu/drm/exynos/exynos_drm_mic.c | 2 ++
 drivers/gpu/drm/mediatek/mtk_hdmi.c | 2 ++
 drivers/gpu/drm/sti/sti_dvo.c   | 2 ++
 3 files changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_mic.c 
b/drivers/gpu/drm/exynos/exynos_drm_mic.c
index a0def0be6d65..7175ecda36e8 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_mic.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_mic.c
@@ -390,6 +390,8 @@ static int exynos_mic_bind(struct device *dev, struct 
device *master,
mic->bridge.funcs = &mic_bridge_funcs;
mic->bridge.of_node = dev->of_node;
mic->bridge.driver_private = mic;
+   mic->bridge.encoder_type = DRM_MODE_ENCODER_DSI;
+
ret = drm_bridge_add(&mic->bridge);
if (ret)
DRM_ERROR("mic: Failed to add MIC to the global bridge list\n");
diff --git a/drivers/gpu/drm/mediatek/mtk_hdmi.c 
b/drivers/gpu/drm/mediatek/mtk_hdmi.c
index 5ca1b0fbf937..bb5cf594c721 100644
--- a/drivers/gpu/drm/mediatek/mtk_hdmi.c
+++ b/drivers/gpu/drm/mediatek/mtk_hdmi.c
@@ -1712,6 +1712,8 @@ static int mtk_drm_hdmi_probe(struct platform_device 
*pdev)
 
hdmi->bridge.funcs = &mtk_hdmi_bridge_funcs;
hdmi->bridge.of_node = pdev->dev.of_node;
+   hdmi->bridge.encoder_type = DRM_MODE_ENCODER_TMDS;
+
ret = drm_bridge_add(&hdmi->bridge);
if (ret) {
dev_err(dev, "failed to add bridge, ret = %d\n", ret);
diff --git a/drivers/gpu/drm/sti/sti_dvo.c b/drivers/gpu/drm/sti/sti_dvo.c
index 411dc6ec976e..8912c932078f 100644
--- a/drivers/gpu/drm/sti/sti_dvo.c
+++ b/drivers/gpu/drm/sti/sti_dvo.c
@@ -472,6 +472,8 @@ static int sti_dvo_bind(struct device *dev, struct device 
*master, void *data)
bridge->driver_private = dvo;
bridge->funcs = &sti_dvo_bridge_funcs;
bridge->of_node = dvo->dev.of_node;
+   bridge->encoder_type = DRM_MODE_ENCODER_LVDS;
+
err = drm_bridge_add(bridge);
if (err) {
DRM_ERROR("Failed to add bridge\n");
-- 
Regards,

Laurent Pinchart



[PATCH v3 07/13] drm: bridge: vga-dac: Add adi,adv7123 compatible string

2016-11-29 Thread Laurent Pinchart
The ADV7123 is a transparent VGA DAC. Unlike dumb VGA DACs it can be
controlled through a power save pin, and requires a power supply.
However, on most boards where the device is used neither the power save
signal nor the power supply are controllable.

To avoid developing a separate device-specific driver add an
"adi,adv7123" compatible entry to the dumb-vga-dac driver. This will
allow supporting most ADV7123-based boards easily, while allowing future
development of an adv7123 driver when needed without breaking backward
compatibility.

Signed-off-by: Laurent Pinchart 
---
 drivers/gpu/drm/bridge/dumb-vga-dac.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/bridge/dumb-vga-dac.c 
b/drivers/gpu/drm/bridge/dumb-vga-dac.c
index afec232185a7..b33e3f829e4f 100644
--- a/drivers/gpu/drm/bridge/dumb-vga-dac.c
+++ b/drivers/gpu/drm/bridge/dumb-vga-dac.c
@@ -204,6 +204,7 @@ static int dumb_vga_remove(struct platform_device *pdev)
 
 static const struct of_device_id dumb_vga_match[] = {
{ .compatible = "dumb-vga-dac" },
+   { .compatible = "adi,adv7123" },
{},
 };
 MODULE_DEVICE_TABLE(of, dumb_vga_match);
-- 
Regards,

Laurent Pinchart



[PATCH v3 06/13] drm: bridge: Add LVDS encoder driver

2016-11-29 Thread Laurent Pinchart
The LVDS encoder driver is a DRM bridge driver that supports the
parallel to LVDS encoders that don't require any configuration. The
driver thus doesn't interact with the device, but creates an LVDS
connector for the panel and exposes its size and timing based on
information retrieved from DT.

Signed-off-by: Laurent Pinchart 
---
 drivers/gpu/drm/bridge/Kconfig|   8 ++
 drivers/gpu/drm/bridge/Makefile   |   1 +
 drivers/gpu/drm/bridge/lvds-encoder.c | 217 ++
 3 files changed, 226 insertions(+)
 create mode 100644 drivers/gpu/drm/bridge/lvds-encoder.c

diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
index bd6acc829f97..a4d9657c9477 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -24,6 +24,14 @@ config DRM_DUMB_VGA_DAC
help
  Support for RGB to VGA DAC based bridges
 
+config DRM_LVDS_ENCODER
+   tristate "Transparent parallel to LVDS encoder support"
+   depends on OF
+   select DRM_KMS_HELPER
+   help
+ Support for transparent parallel to LVDS encoders that don't require
+ any configuration.
+
 config DRM_DW_HDMI
tristate
select DRM_KMS_HELPER
diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
index 97ed1a5fea9a..7e0bf849c30d 100644
--- a/drivers/gpu/drm/bridge/Makefile
+++ b/drivers/gpu/drm/bridge/Makefile
@@ -2,6 +2,7 @@ ccflags-y := -Iinclude/drm
 
 obj-$(CONFIG_DRM_ANALOGIX_ANX78XX) += analogix-anx78xx.o
 obj-$(CONFIG_DRM_DUMB_VGA_DAC) += dumb-vga-dac.o
+obj-$(CONFIG_DRM_LVDS_ENCODER) += lvds-encoder.o
 obj-$(CONFIG_DRM_DW_HDMI) += dw-hdmi.o
 obj-$(CONFIG_DRM_DW_HDMI_AHB_AUDIO) += dw-hdmi-ahb-audio.o
 obj-$(CONFIG_DRM_NXP_PTN3460) += nxp-ptn3460.o
diff --git a/drivers/gpu/drm/bridge/lvds-encoder.c 
b/drivers/gpu/drm/bridge/lvds-encoder.c
new file mode 100644
index ..576770ea8d7e
--- /dev/null
+++ b/drivers/gpu/drm/bridge/lvds-encoder.c
@@ -0,0 +1,217 @@
+/*
+ * Copyright (C) 2016 Laurent Pinchart 
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+
+struct lvds_encoder {
+   struct device *dev;
+
+   struct drm_bridge bridge;
+   struct drm_connector connector;
+   struct drm_panel *panel;
+};
+
+static inline struct lvds_encoder *
+drm_bridge_to_lvds_encoder(struct drm_bridge *bridge)
+{
+   return container_of(bridge, struct lvds_encoder, bridge);
+}
+
+static inline struct lvds_encoder *
+drm_connector_to_lvds_encoder(struct drm_connector *connector)
+{
+   return container_of(connector, struct lvds_encoder, connector);
+}
+
+static int lvds_connector_get_modes(struct drm_connector *connector)
+{
+   struct lvds_encoder *lvds = drm_connector_to_lvds_encoder(connector);
+
+   return drm_panel_get_modes(lvds->panel);
+}
+
+static const struct drm_connector_helper_funcs lvds_connector_helper_funcs = {
+   .get_modes = lvds_connector_get_modes,
+};
+
+static enum drm_connector_status
+lvds_connector_detect(struct drm_connector *connector, bool force)
+{
+   return connector_status_connected;
+}
+
+static const struct drm_connector_funcs lvds_connector_funcs = {
+   .dpms = drm_atomic_helper_connector_dpms,
+   .reset = drm_atomic_helper_connector_reset,
+   .detect = lvds_connector_detect,
+   .fill_modes = drm_helper_probe_single_connector_modes,
+   .destroy = drm_connector_cleanup,
+   .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
+   .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
+};
+
+static int lvds_encoder_attach(struct drm_bridge *bridge)
+{
+   struct lvds_encoder *lvds = drm_bridge_to_lvds_encoder(bridge);
+   struct drm_connector *connector = &lvds->connector;
+   int ret;
+
+   if (!bridge->encoder) {
+   DRM_ERROR("Missing encoder\n");
+   return -ENODEV;
+   }
+
+   drm_connector_helper_add(connector, &lvds_connector_helper_funcs);
+
+   ret = drm_connector_init(bridge->dev, connector, &lvds_connector_funcs,
+DRM_MODE_CONNECTOR_LVDS);
+   if (ret) {
+   DRM_ERROR("Failed to initialize connector\n");
+   return ret;
+   }
+
+   drm_mode_connector_attach_encoder(&lvds->connector, bridge->encoder);
+
+   ret = drm_panel_attach(lvds->panel, &lvds->connector);
+   if (ret < 0)
+   return ret;
+
+   return 0;
+}
+
+static void lvds_encoder_detach(struct drm_bridge *bridge)
+{
+   struct lvds_encoder *lvds = drm_bridge_to_lvds_encoder(bridge);
+
+   drm_panel_detach(lvds->panel);
+}
+
+static void lvds_encoder_pre_enable(str

[PATCH v3 05/13] drm: bridge: Add LVDS encoder DT bindings

2016-11-29 Thread Laurent Pinchart
The DT bindings support parallel to LVDS encoders that don't require any
configuration, similarly to the dumb VGA DAC DT bindings.

Signed-off-by: Laurent Pinchart 
Acked-by: Rob Herring 
---
 .../bindings/display/bridge/lvds-transmitter.txt   | 64 ++
 1 file changed, 64 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt

diff --git 
a/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt 
b/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt
new file mode 100644
index ..fd39ad34c383
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt
@@ -0,0 +1,64 @@
+Parallel to LVDS Encoder
+
+
+This binding supports the parallel to LVDS encoders that don't require any
+configuration.
+
+LVDS is a physical layer specification defined in ANSI/TIA/EIA-644-A. Multiple
+incompatible data link layers have been used over time to transmit image data
+to LVDS panels. This binding targets devices compatible with the following
+specifications only.
+
+[JEIDA] "Digital Interface Standards for Monitor", JEIDA-59-1999, February
+1999 (Version 1.0), Japan Electronic Industry Development Association (JEIDA)
+[LDI] "Open LVDS Display Interface", May 1999 (Version 0.95), National
+Semiconductor
+[VESA] "VESA Notebook Panel Standard", October 2007 (Version 1.0), Video
+Electronics Standards Association (VESA)
+
+Those devices have been marketed under the FPD-Link and FlatLink brand names
+among others.
+
+
+Required properties:
+
+- compatible: Must be "lvds-encoder"
+
+Required nodes:
+
+This device has two video ports. Their connections are modeled using the OF
+graph bindings specified in Documentation/devicetree/bindings/graph.txt.
+
+- Video port 0 for parallel input
+- Video port 1 for LVDS output
+
+
+Example
+---
+
+lvds-encoder {
+   compatible = "lvds-encoder";
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   ports {
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   port@0 {
+   reg = <0>;
+
+   lvds_enc_in: endpoint {
+   remote-endpoint = <&display_out_rgb>;
+   };
+   };
+
+   port@1 {
+   reg = <1>;
+
+   lvds_enc_out: endpoint {
+   remote-endpoint = <&lvds_panel_in>;
+   };
+   };
+   };
+};
-- 
Regards,

Laurent Pinchart



[PATCH v3 03/13] drm: bridge: Link encoder and bridge in core code

2016-11-29 Thread Laurent Pinchart
Instead of linking encoders and bridges in every driver (and getting it
wrong half of the time, as many drivers forget to set the drm_bridge
encoder pointer), do so in core code. The drm_bridge_attach() function
needs the encoder and optional previous bridge to perform that task,
update all the callers.

Signed-off-by: Laurent Pinchart 
---
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c   |  4 +-
 drivers/gpu/drm/bridge/analogix/analogix_dp_core.c |  4 +-
 drivers/gpu/drm/bridge/dw-hdmi.c   |  3 +-
 drivers/gpu/drm/drm_bridge.c   | 46 --
 drivers/gpu/drm/drm_simple_kms_helper.c|  4 +-
 drivers/gpu/drm/exynos/exynos_dp.c |  5 +--
 drivers/gpu/drm/exynos/exynos_drm_dsi.c|  6 +--
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c  |  5 +--
 drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c   |  5 +--
 drivers/gpu/drm/imx/imx-ldb.c  |  6 +--
 drivers/gpu/drm/imx/parallel-display.c |  4 +-
 drivers/gpu/drm/mediatek/mtk_dpi.c |  8 ++--
 drivers/gpu/drm/mediatek/mtk_dsi.c | 24 ++-
 drivers/gpu/drm/mediatek/mtk_hdmi.c| 11 +++---
 drivers/gpu/drm/msm/dsi/dsi_manager.c  | 17 +---
 drivers/gpu/drm/msm/edp/edp_bridge.c   |  2 +-
 drivers/gpu/drm/msm/hdmi/hdmi_bridge.c |  2 +-
 drivers/gpu/drm/rcar-du/rcar_du_hdmienc.c  |  5 +--
 drivers/gpu/drm/sti/sti_dvo.c  |  3 +-
 drivers/gpu/drm/sti/sti_hda.c  |  3 +-
 drivers/gpu/drm/sti/sti_hdmi.c |  3 +-
 drivers/gpu/drm/sun4i/sun4i_rgb.c  | 13 +++---
 include/drm/drm_bridge.h   |  3 +-
 23 files changed, 83 insertions(+), 103 deletions(-)

diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c 
b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c
index 6119b5085501..e7799b6ee829 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c
@@ -230,9 +230,7 @@ static int atmel_hlcdc_attach_endpoint(struct drm_device 
*dev,
of_node_put(np);
 
if (bridge) {
-   output->encoder.bridge = bridge;
-   bridge->encoder = &output->encoder;
-   ret = drm_bridge_attach(dev, bridge);
+   ret = drm_bridge_attach(&output->encoder, bridge, NULL);
if (!ret)
return 0;
}
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c 
b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
index 6e0447f329a2..1835f1fdad19 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
@@ -1227,12 +1227,10 @@ static int analogix_dp_create_bridge(struct drm_device 
*drm_dev,
 
dp->bridge = bridge;
 
-   dp->encoder->bridge = bridge;
bridge->driver_private = dp;
-   bridge->encoder = dp->encoder;
bridge->funcs = &analogix_dp_bridge_funcs;
 
-   ret = drm_bridge_attach(drm_dev, bridge);
+   ret = drm_bridge_attach(dp->encoder, bridge, NULL);
if (ret) {
DRM_ERROR("failed to attach drm bridge\n");
return -EINVAL;
diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c b/drivers/gpu/drm/bridge/dw-hdmi.c
index b71088dab268..432e0e3fff72 100644
--- a/drivers/gpu/drm/bridge/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/dw-hdmi.c
@@ -1841,13 +1841,12 @@ static int dw_hdmi_register(struct drm_device *drm, 
struct dw_hdmi *hdmi)
hdmi->bridge = bridge;
bridge->driver_private = hdmi;
bridge->funcs = &dw_hdmi_bridge_funcs;
-   ret = drm_bridge_attach(drm, bridge);
+   ret = drm_bridge_attach(encoder, bridge, NULL);
if (ret) {
DRM_ERROR("Failed to initialize bridge with drm\n");
return -EINVAL;
}
 
-   encoder->bridge = bridge;
hdmi->connector.polled = DRM_CONNECTOR_POLL_HPD;
 
drm_connector_helper_add(&hdmi->connector,
diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
index 0ee052b7c21a..850bd6509ef1 100644
--- a/drivers/gpu/drm/drm_bridge.c
+++ b/drivers/gpu/drm/drm_bridge.c
@@ -26,6 +26,7 @@
 #include 
 
 #include 
+#include 
 
 /**
  * DOC: overview
@@ -92,32 +93,53 @@ void drm_bridge_remove(struct drm_bridge *bridge)
 EXPORT_SYMBOL(drm_bridge_remove);
 
 /**
- * drm_bridge_attach - associate given bridge to our DRM device
+ * drm_bridge_attach - attach the bridge to an encoder's chain
  *
- * @dev: DRM device
- * @bridge: bridge control structure
+ * @encoder: DRM encoder
+ * @bridge: bridge to attach
+ * @previous: previous bridge in the chain (optional)
  *
- * Called by a kms driver to link one of our encoder/bridge to the given
- * bridge.
+ * Called by a kms driver to link the bridge to an encoder's chain. The 
previous
+ * argument specifies the previous bridge in the 

[PATCH v3 00/13] R-Car DU: Use drm bridge API

2016-11-29 Thread Laurent Pinchart
Hello,

This patch series replaces the custom external encoders support implementation
in the R-Car DU driver with code based on the DRM bridge API.

While the overall diffstat isn't impressive, the rcar-du-drm driver gets
notably thinner in the process:

9 files changed, 64 insertions(+), 384 deletions(-)

This is offset by a reusable driver for LVDS encoders along with the
corresponding DT bindings (+ 218 lines).

The series starts with a few cleanups. Patch 01/13 fixes all drivers to
include  where needed, allowing to remove inclusion of the
header from  (there are plenty of other headers that need
similar cleanup, volunteers are welcome). Patch 02/13 then includes
 in  to fix a compilation warning.

The next patche moves copied&pasted code from a large number of drivers to the
DRM bridge core drm_bridge_attach() function. The function now links bridges
to encoders or other bridges instead of letting the caller perform that
operation. Patch 04/13 can then detach bridges in the encoder cleanup
function, removing the need to call drm_bridge_detach() explicitly in drivers
(all but one of the drivers that use bridges were missing that call).

Patches 05/13 and 06/13 add DT bindings for LVDS encoders with a corresponding
driver. It supports "dumb" LVDS encoders only, similarly to the dumb-vga-dac
driver. One notable difference, though, is that LVDS encoders can't be purely
passive, and thus require at least one power supply (and usually multiple of
them) and have a few control GPIOs (most notably to control reset, power down,
clock polarity and/or LVDS slew rate). However, on many systems those encoders
are integrated in such a way that the control pins are pulled up or down
appropriately and the power supplies are either always on or shared with other
display components that make them operate as if they were always on. For that
reason a common drivers for those systems is useful, with simple DT bindings
that don't try to cover any device-specific control pin or power supply.

To ensure backward compatibility most LVDS encoders should *not* use the
common simple "lvds-encoder" compatible string, but should instead define a
device-specific compatible string that can then be added to the lvds-encoder
driver (patch 08/13). This way, when the need to control pins or supplies will
arise, a new driver can be developed matching on the device-specific
compatible string, which will then be removed from the simple driver. Existing
systems will migrate transparently without requiring a change to their device
tree.

A similar reasoning applies to VGA DACs, leading to the addition of the
"adi,adv7123" compatible string to the dumb-vga-dac driver's OF match table in
patch 07/13.

Patch 09/13 adds a new type field to the drm_bridge structure to inform bridge
users of the bridge type. This is useful for display driver to create a DRM
encoder of the appropriate type without having to resort to heuristics.
Patches 10/13 and 11/13 update all bridge drivers to initialize the new field
to the appropriate value.

Patches 12/13 and 13/13 finally migrate the rcar-du-drm driver to the DRM
bridge API, removing the custom VGA DAC implementation in patch 12/13 and the
table of bridge compatible strings used to find the encoder type in patch
13/13.

Changes since v2:

- Added patches 01/13 to 04/13.
- Rebased on top of latest DRM master branch

Changes since v1:

- The patches have been rebased on top of the "[PATCH v2 00/13] R-Car DU: Add
  support for LVDS mode selection" series.
- The LVDS encoder DT bindings have been split from the LVDS encoder driver
  into a separate patch.
- The wording of the DRM bridge documentation new property has been updated.

Laurent Pinchart (13):
  drm: Don't include  in 
  drm: Fix compilation warning caused by static inline forward
declaration
  drm: bridge: Link encoder and bridge in core code
  drm: bridge: Detach bridge from encoder at encoder cleanup time
  drm: bridge: Add LVDS encoder DT bindings
  drm: bridge: Add LVDS encoder driver
  drm: bridge: vga-dac: Add adi,adv7123 compatible string
  drm: bridge: lvds-encoder: Add thine,thc63lvdm83d compatible string
  drm: Add encoder_type field to the drm_bridge structure
  drm: bridge: Set bridges' encoder type
  drm: Set on-chip bridges' encoder type
  drm: rcar-du: Replace manual bridge implementation with DRM bridge
  drm: rcar-du: Initialize encoder's type based on the bridge's type

 .../bindings/display/bridge/lvds-transmitter.txt   |  64 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h   |   1 +
 drivers/gpu/drm/ast/ast_drv.h  |   1 +
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c   |   4 +-
 drivers/gpu/drm/bochs/bochs.h  |   1 +
 drivers/gpu/drm/bridge/Kconfig |   8 +
 drivers/gpu/drm/bridge/Makefile|   1 +
 drivers/gpu/drm/bridge/adv7511/adv7511_drv.c   |   1 +
 drivers/gpu/drm/bridge/analogix-anx78xx.c  |   1 +
 drivers/gpu/d

[PATCH v3 01/13] drm: Don't include in

2016-11-29 Thread Laurent Pinchart
 used to define most of the in-kernel KMS API. It has
now been split into separate files for each object type, but still
includes most other KMS headers to avoid breaking driver compilation.

As a step towards fixing that problem, remove the inclusion of
 from  and include it instead where
appropriate. Also remove the forward declarations of the drm_encoder and
drm_encoder_helper_funcs structures from  as they're not
needed in the header.

 now has to include  and contain a
forward declaration of struct drm_encoder in order to allow including it
as the first header in a compilation unit.

Signed-off-by: Laurent Pinchart 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h| 1 +
 drivers/gpu/drm/ast/ast_drv.h   | 1 +
 drivers/gpu/drm/bochs/bochs.h   | 1 +
 drivers/gpu/drm/cirrus/cirrus_drv.h | 1 +
 drivers/gpu/drm/drm_connector.c | 1 +
 drivers/gpu/drm/drm_crtc_helper.c   | 1 +
 drivers/gpu/drm/drm_edid.c  | 1 +
 drivers/gpu/drm/drm_mode_config.c   | 1 +
 drivers/gpu/drm/drm_of.c| 1 +
 drivers/gpu/drm/drm_plane_helper.c  | 1 +
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h   | 2 ++
 drivers/gpu/drm/gma500/psb_intel_drv.h  | 1 +
 drivers/gpu/drm/i915/intel_drv.h| 1 +
 drivers/gpu/drm/mgag200/mgag200_drv.h   | 1 +
 drivers/gpu/drm/nouveau/nouveau_connector.h | 1 +
 drivers/gpu/drm/qxl/qxl_drv.h   | 1 +
 drivers/gpu/drm/radeon/radeon_mode.h| 1 +
 drivers/gpu/drm/rcar-du/rcar_du_encoder.h   | 1 +
 drivers/gpu/drm/shmobile/shmob_drm_crtc.h   | 1 +
 drivers/gpu/drm/tegra/drm.h | 1 +
 drivers/gpu/drm/vc4/vc4_drv.h   | 2 ++
 drivers/gpu/drm/virtio/virtgpu_drv.h| 1 +
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.h | 1 +
 include/drm/drm_crtc.h  | 3 ---
 include/drm/drm_encoder.h   | 3 +++
 include/drm/drm_encoder_slave.h | 1 +
 include/drm/drm_modeset_helper_vtables.h| 1 +
 27 files changed, 30 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
index 1e23334b07fb..fac06064a8f5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
@@ -32,6 +32,7 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
index 908011d2c8f5..6f3b6f50cf52 100644
--- a/drivers/gpu/drm/ast/ast_drv.h
+++ b/drivers/gpu/drm/ast/ast_drv.h
@@ -28,6 +28,7 @@
 #ifndef __AST_DRV_H__
 #define __AST_DRV_H__
 
+#include 
 #include 
 
 #include 
diff --git a/drivers/gpu/drm/bochs/bochs.h b/drivers/gpu/drm/bochs/bochs.h
index 32dfe418cc98..f626bab7f5e3 100644
--- a/drivers/gpu/drm/bochs/bochs.h
+++ b/drivers/gpu/drm/bochs/bochs.h
@@ -4,6 +4,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #include 
diff --git a/drivers/gpu/drm/cirrus/cirrus_drv.h 
b/drivers/gpu/drm/cirrus/cirrus_drv.h
index 2188d6b61b3e..b59aeef4635a 100644
--- a/drivers/gpu/drm/cirrus/cirrus_drv.h
+++ b/drivers/gpu/drm/cirrus/cirrus_drv.h
@@ -13,6 +13,7 @@
 
 #include 
 
+#include 
 #include 
 
 #include 
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index b5c6a8ee831e..5f1e1f190d30 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -23,6 +23,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "drm_crtc_internal.h"
 #include "drm_internal.h"
diff --git a/drivers/gpu/drm/drm_crtc_helper.c 
b/drivers/gpu/drm/drm_crtc_helper.c
index 5d2cb138eba6..b3fc23313cc3 100644
--- a/drivers/gpu/drm/drm_crtc_helper.c
+++ b/drivers/gpu/drm/drm_crtc_helper.c
@@ -36,6 +36,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 7eec18925b70..a9e3cc3990c1 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -35,6 +35,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #define version_greater(edid, maj, min) \
diff --git a/drivers/gpu/drm/drm_mode_config.c 
b/drivers/gpu/drm/drm_mode_config.c
index 2735a5847ffa..09b1d8f267a6 100644
--- a/drivers/gpu/drm/drm_mode_config.c
+++ b/drivers/gpu/drm/drm_mode_config.c
@@ -20,6 +20,7 @@
  * OF THIS SOFTWARE.
  */
 
+#include 
 #include 
 #include 
 
diff --git a/drivers/gpu/drm/drm_of.c b/drivers/gpu/drm/drm_of.c
index 47848ed8ca48..b5f2f0fece99 100644
--- a/drivers/gpu/drm/drm_of.c
+++ b/drivers/gpu/drm/drm_of.c
@@ -4,6 +4,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 static void drm_release_of(struct device *dev, void *data)
diff --git a/drivers/gpu/drm/drm_plane_helper.c 
b/drivers/gpu/drm/drm_plane_helper.c
index 7a7dddf604d7..191a5f095cf9 100644
--- a/drivers/gpu/drm/drm_plane_helper.c
+++ b/drivers/gpu/drm/drm_plane_helper.c
@@ -29,6 +29,7 @@
 #include 
 #include 
 #include 
+#include 
 #inclu

[PATCH v3 02/13] drm: Fix compilation warning caused by static inline forward declaration

2016-11-29 Thread Laurent Pinchart
The drm_crtc_mask() function used in  is a static
inline defined in . If the first header is included in a
compilation unit without the second one, the following compilation
warning will be issued.

In file included from /drivers/gpu/drm/drm_bridge.c:29:0:
/include/drm/drm_encoder.h:192:95: warning: ‘drm_crtc_mask’ used but 
never defined
 static inline uint32_t drm_crtc_mask(const struct drm_crtc *crtc);

Fix this by including the header defining the function instead of using
a forward declaration.

Signed-off-by: Laurent Pinchart 
---
 include/drm/drm_encoder.h | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/include/drm/drm_encoder.h b/include/drm/drm_encoder.h
index cec6ac45c6cc..5f58f65344e0 100644
--- a/include/drm/drm_encoder.h
+++ b/include/drm/drm_encoder.h
@@ -25,6 +25,7 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -191,9 +192,6 @@ static inline unsigned int drm_encoder_index(struct 
drm_encoder *encoder)
return encoder->index;
 }
 
-/* FIXME: We have an include file mess still, drm_crtc.h needs untangling. */
-static inline uint32_t drm_crtc_mask(const struct drm_crtc *crtc);
-
 /**
  * drm_encoder_crtc_ok - can a given crtc drive a given encoder?
  * @encoder: encoder to test
-- 
Regards,

Laurent Pinchart



Re: [PATCH v3 2/4] thermal: rcar_gen3_thermal: Add R-Car Gen3 thermal driver

2016-11-29 Thread Geert Uytterhoeven
Hi Wolfram,

On Mon, Nov 28, 2016 at 10:09 PM, Wolfram Sang
 wrote:
> --- /dev/null
> +++ b/drivers/thermal/rcar_gen3_thermal.c

> +/* Structure for thermal temperature calculation */
> +struct equation_coefs {
> +   long a1;
> +   long b1;
> +   long a2;
> +   long b2;

Why long? Long has a different size for 32-bit and 64-bit platforms.
I know this is a driver for arm64, but if you need 64-bits, you want to make
this clear using s64, or long long.

> +static inline u32 rcar_gen3_thermal_read(struct rcar_gen3_thermal_tsc *tsc, 
> u32 reg)

What a long function name...

> +{
> +   return ioread32(tsc->base + reg);
> +}
> +
> +static inline void rcar_gen3_thermal_write(struct rcar_gen3_thermal_tsc *tsc,
> +u32 reg, u32 data)
> +{
> +   iowrite32(data, tsc->base + reg);
> +}

... so using the "convenience" wrappers is more (typing) work than using
io{read,write}32() directly?

> +static void _linear_coefficient_calculation(struct rcar_gen3_thermal_tsc 
> *tsc,
> +   int *ptat, int *thcode)
> +{
> +   int tj_2;
> +   long a1, b1;
> +   long a2, b2;
> +   long a1_num, a1_den;
> +   long a2_num, a2_den;

s64?

> +   tj_2 = (CODETSD((ptat[1] - ptat[2]) * 137)
> +   / (ptat[0] - ptat[2])) - CODETSD(41);

Isn't "* 1000" easier to read then CODETSD()?

> +   /* calculate coefficients for linear equation */
> +   a1_num = CODETSD(thcode[1] - thcode[2]);
> +   a1_den = tj_2 - TJ_3;
> +   a1 = (1 * a1_num) / a1_den;
> +   b1 = (1 * thcode[2]) - ((a1 * TJ_3) / 1000);

Rounding needed for / 1000?

> +   a2_num = CODETSD(thcode[1] - thcode[0]);
> +   a2_den = tj_2 - TJ_1;
> +   a2 = (1 * a2_num) / a2_den;
> +   b2 = (1 * thcode[0]) - ((a2 * TJ_1) / 1000);

Idem.

> +static int rcar_gen3_thermal_probe(struct platform_device *pdev)
> +{
> +   struct rcar_gen3_thermal_priv *priv;
> +   struct device *dev = &pdev->dev;
> +   struct resource *res;
> +   struct thermal_zone_device *zone;
> +   int ret, i;

unsigned int i;

> +   const struct rcar_gen3_thermal_data *match_data = 
> of_device_get_match_data(dev);
> +
> +   /* default values if FUSEs are missing */
> +   int ptat[3] = { 2351, 1509, 435 };

unsigned?

> +   int thcode[TSC_MAX_NUM][3] = {

unsigned?

> +   { 3248, 2800, 2221 },
> +   { 3245, 2795, 2216 },
> +   { 3250, 2805, 2237 },
> +   };
> +
> +   priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> +   if (!priv)
> +   return -ENOMEM;
> +
> +   platform_set_drvdata(pdev, priv);
> +
> +   pm_runtime_enable(dev);
> +   pm_runtime_get_sync(dev);
> +
> +   for (i = 0; i < TSC_MAX_NUM; i++) {
> +   struct rcar_gen3_thermal_tsc *tsc;
> +
> +   tsc = devm_kzalloc(dev, sizeof(*tsc), GFP_KERNEL);
> +   if (!tsc)
> +   return -ENOMEM;

Missing pm_runtime_put() etc.?

ret = -ENOMEM;
goto error_unregister;

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds


Re: [PATCH v3 2/4] thermal: rcar_gen3_thermal: Add R-Car Gen3 thermal driver

2016-11-29 Thread Wolfram Sang
Hi Eduardo,

> No commit message ? :-( generally speaking here, it is a good practice
> to describe what you are doing, what you have considered, and what you
> have tested, just for the sake of code history/documentation.

OK, can do.

> > +   spin_lock_irqsave(&tsc->lock, flags);
> 
> Why do you need a full blown spin lock irqsave? Can this locking be
> solved by a simple mutex?

Currently, it can be a mutex, yes. This is a "left-over" from the
version which had interrupt support. I am not fully done with
refactoring the irqs, but I thought it is likely we need this lock then
again, so I chose to leave it. I can use a mutex for now if you prefer.

> 
> > +static void r8a7795_thermal_init(struct rcar_gen3_thermal_tsc *tsc)
> > +{
> > +   unsigned long flags;
> > +
> > +   spin_lock_irqsave(&tsc->lock, flags);
> > +
> > +   rcar_gen3_thermal_write(tsc, REG_GEN3_CTSR,  CTSR_THBGR);
> > +   rcar_gen3_thermal_write(tsc, REG_GEN3_CTSR,  0x0);
> > +
> > +   udelay(1000);
> 
> Why do you disable irqs, then busyloop for 1ms?

Uh yes, this is ugly. I don't think we need to lock during init, but
I'll double check later.

> > +   /* default values if FUSEs are missing */
> > +   int ptat[3] = { 2351, 1509, 435 };
> > +   int thcode[TSC_MAX_NUM][3] = {
> > +   { 3248, 2800, 2221 },
> > +   { 3245, 2795, 2216 },
> > +   { 3250, 2805, 2237 },
> > +   };
> 
> are these fuses valid for both? 7796 and 7797? or would you need a
> different array for each version?

According to the information I have today, it is the same array. And all
later versions are promised to have fuse registers. This is already
documented, but such HW is not available to me currently.

Thanks for the quick review, very much appreciated!

   Wolfram



signature.asc
Description: PGP signature


Re: [PATCH v2 01/13] devicetree/bindings: display: Document common panel properties

2016-11-29 Thread Laurent Pinchart
Hi Rob,

On Tuesday 22 Nov 2016 11:36:55 Laurent Pinchart wrote:
> On Monday 21 Nov 2016 10:48:15 Rob Herring wrote:
> > On Sat, Nov 19, 2016 at 05:28:01AM +0200, Laurent Pinchart wrote:
> >> Document properties common to several display panels in a central
> >> location that can be referenced by the panel device tree bindings.
> > 
> > Looks good. Just one comment...
> > 
> > [...]
> > 
> >> +Connectivity
> >> +
> >> +
> >> +- ports: Panels receive video data through one or multiple connections.
> >> While
> >> +  the nature of those connections is specific to the panel type, the
> >> +  connectivity is expressed in a standard fashion using ports as
> >> specified in
> >> +  the device graph bindings defined in
> >> +  Documentation/devicetree/bindings/graph.txt.
> > 
> > We allow panels to either use graph binding or be a child of the display
> > controller.
> 
> I knew that some display controllers use a phandle to the panel (see the
> fsl,panel and nvidia,panel properties), but I didn't know we had panels as
> children of display controller nodes. I don't think we should allow that for
> anything but DSI panels, as the DT hierarchy is based on control buses. Are
> you sure we have other panels instantiated through that mechanism ?

Ping ?

Please note that this file documents properties common to multiple panel DT 
bindings, but in no way makes it mandatory to use the OF graph bindings for 
panels. The decision is left to individual bindings.

> > Using the graph is preferred, but in the simple cases just a child node is
> > sufficient. This should be described here or somewhere in this doc.

-- 
Regards,

Laurent Pinchart