On Fri, Aug 30, 2013 at 2:59 AM, Rahul Sharma <rahul.sharma@xxxxxxxxxxx> wrote:
> Before hdmiphy operation like config, start etc, hdmiphy
> bit in PMU block should be enabled. Earlier this happens
> in hdmi drvier through a dummy "hdmiphy" clock.
s/drvier/driver/
>
> Signed-off-by: Rahul Sharma <rahul.sharma@xxxxxxxxxxx>
> ---
> .../devicetree/bindings/video/exynos_hdmiphy.txt | 6 ++
> drivers/gpu/drm/exynos/exynos_drm_hdmi.h | 2 +
> drivers/gpu/drm/exynos/exynos_hdmi.c | 2 +
> drivers/gpu/drm/exynos/exynos_hdmiphy.c | 82 ++++++++++++++++++++
> 4 files changed, 92 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/video/exynos_hdmiphy.txt b/Documentation/devicetree/bindings/video/exynos_hdmiphy.txt
> index 162f641..f6bf096 100644
> --- a/Documentation/devicetree/bindings/video/exynos_hdmiphy.txt
> +++ b/Documentation/devicetree/bindings/video/exynos_hdmiphy.txt
> @@ -6,10 +6,16 @@ Required properties:
> 2) "samsung,exynos4210-hdmiphy".
> 3) "samsung,exynos4212-hdmiphy".
> - reg: I2C address of the hdmiphy device.
> +- phy-power-control: this child node represents phy power control
> + register which is inside the pmu block (power management unit).
Should you include devicetree-discuss on this?
>
> Example:
>
> hdmiphy {
> compatible = "samsung,exynos4210-hdmiphy";
> reg = <0x38>;
> +
> + phy-power-control {
> + reg = <0x10040700 0x04>;
> + };
> };
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_hdmi.h b/drivers/gpu/drm/exynos/exynos_drm_hdmi.h
> index 1c839f8..9a14f96 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_hdmi.h
> +++ b/drivers/gpu/drm/exynos/exynos_drm_hdmi.h
> @@ -68,6 +68,8 @@ void exynos_mixer_ops_register(struct exynos_mixer_ops *ops);
> int exynos_hdmiphy_driver_register(void);
> void exynos_hdmiphy_driver_unregister(void);
>
> +void exynos_hdmiphy_poweron(struct device *dev);
> +void exynos_hdmiphy_poweroff(struct device *dev);
> void exynos_hdmiphy_enable(struct device *dev);
> void exynos_hdmiphy_disable(struct device *dev);
> int exynos_hdmiphy_check_mode(struct device *dev,
> diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c
> index cd1d921..a6234fc 100644
> --- a/drivers/gpu/drm/exynos/exynos_hdmi.c
> +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
> @@ -1131,6 +1131,7 @@ static void hdmiphy_poweron(struct hdmi_context *hdata)
> if (hdata->type == HDMI_TYPE14)
> hdmi_reg_writemask(hdata, HDMI_PHY_CON_0, 0,
> HDMI_PHY_POWER_OFF_EN);
> + exynos_hdmiphy_poweron(hdata->hdmiphy_dev);
> }
>
> static void hdmiphy_poweroff(struct hdmi_context *hdata)
> @@ -1138,6 +1139,7 @@ static void hdmiphy_poweroff(struct hdmi_context *hdata)
> if (hdata->type == HDMI_TYPE14)
> hdmi_reg_writemask(hdata, HDMI_PHY_CON_0, ~0,
> HDMI_PHY_POWER_OFF_EN);
> + exynos_hdmiphy_poweroff(hdata->hdmiphy_dev);
Shouldn't the phy power off before HDMI?
> }
>
> static void hdmi_conf_apply(struct hdmi_context *hdata)
> diff --git a/drivers/gpu/drm/exynos/exynos_hdmiphy.c b/drivers/gpu/drm/exynos/exynos_hdmiphy.c
> index 82daa42..b1b8a0f 100644
> --- a/drivers/gpu/drm/exynos/exynos_hdmiphy.c
> +++ b/drivers/gpu/drm/exynos/exynos_hdmiphy.c
> @@ -30,6 +30,9 @@ struct hdmiphy_context {
> struct device *dev;
> struct hdmiphy_config *current_conf;
>
> + /* hdmiphy resources */
> + void __iomem *phy_pow_ctrl_reg;
> +
> struct hdmiphy_config *confs;
> unsigned int nr_confs;
> };
> @@ -225,6 +228,49 @@ static struct hdmiphy_config *hdmiphy_find_conf(struct hdmiphy_context *hdata,
> return NULL;
> }
>
> +static int hdmiphy_dt_parse_power_control(struct hdmiphy_context *hdata)
> +{
> + struct device_node *phy_pow_ctrl_node;
> + u32 buf[2];
> + int ret = 0;
> +
> + phy_pow_ctrl_node = of_get_child_by_name(hdata->dev->of_node,
> + "phy-power-control");
> + if (!phy_pow_ctrl_node) {
> + DRM_ERROR("Failed to find phy power control node\n");
> + ret = -ENODEV;
> + goto fail;
You can just return -ENODEV here.
> + }
> +
> + /* reg property holds two informations: addr of pmu register, size */
> + if (of_property_read_u32_array(phy_pow_ctrl_node, "reg",
> + (u32 *)&buf, 2)) {
Maybe I'm missing something, but this looks bad. I think this should be:
of_property_read_u32_array(phy_pow_ctrl_node, "reg", buf, ARRAY_SIZE(buf))
ie: dereferencing buf won't do what you expect it to.
> + DRM_ERROR("faild to get phy power control reg\n");
> + ret = -EINVAL;
> + goto fail;
> + }
> +
> + hdata->phy_pow_ctrl_reg = devm_ioremap(hdata->dev, buf[0], buf[1]);
> + if (!hdata->phy_pow_ctrl_reg) {
> + DRM_ERROR("failed to ioremap phy pmu reg\n");
> + ret = -ENOMEM;
> + goto fail;
> + }
> +
> +fail:
fail seems like the wrong thing to call this, since it's run on
success too :) Maybe "out" would be more appropriate.
> + of_node_put(phy_pow_ctrl_node);
> + return ret;
> +}
> +
> +static inline void hdmiphy_pow_ctrl_reg_writemask(
> + struct hdmiphy_context *hdata,
> + u32 value, u32 mask)
> +{
> + u32 old = readl(hdata->phy_pow_ctrl_reg);
> + value = (value & mask) | (old & ~mask);
> + writel(value, hdata->phy_pow_ctrl_reg);
> +}
> +
> static int hdmiphy_reg_writeb(struct hdmiphy_context *hdata,
> u32 reg_offset, u8 value)
> {
> @@ -353,6 +399,36 @@ void exynos_hdmiphy_disable(struct device *dev)
> }
> }
>
> +void exynos_hdmiphy_poweron(struct device *dev)
> +{
> + struct hdmiphy_context *hdata = dev_get_drvdata(dev);
> +
> + DRM_DEBUG_KMS("[%d]\n", __LINE__);
> +
> + if (!hdata) {
> + DRM_ERROR("Invalid arg: hdmiphy device pointer.\n");
> + return;
> + }
Same comment as the other review. I wonder if you really need this check
> +
> + hdmiphy_pow_ctrl_reg_writemask(hdata, PMU_HDMI_PHY_ENABLE,
> + PMU_HDMI_PHY_CONTROL_MASK);
> +}
> +
> +void exynos_hdmiphy_poweroff(struct device *dev)
> +{
> + struct hdmiphy_context *hdata = dev_get_drvdata(dev);
> +
> + DRM_DEBUG_KMS("[%d]\n", __LINE__);
> +
> + if (!hdata) {
> + DRM_ERROR("Invalid arg: hdmiphy device pointer.\n");
> + return;
> + }
> +
> + hdmiphy_pow_ctrl_reg_writemask(hdata, PMU_HDMI_PHY_DISABLE,
> + PMU_HDMI_PHY_CONTROL_MASK);
> +}
> +
> int exynos_hdmiphy_conf_apply(struct device *dev)
> {
> struct hdmiphy_context *hdata = dev_get_drvdata(dev);
> @@ -413,6 +489,7 @@ static int hdmiphy_i2c_device_probe(struct i2c_client *client,
> struct hdmiphy_context *hdata;
> struct hdmiphy_drv_data *drv;
> const struct of_device_id *match;
> + int ret;
>
> DRM_DEBUG_KMS("[%d]\n", __LINE__);
>
> @@ -436,6 +513,11 @@ static int hdmiphy_i2c_device_probe(struct i2c_client *client,
> hdata->nr_confs = drv->count;
>
> i2c_set_clientdata(client, hdata);
> + ret = hdmiphy_dt_parse_power_control(hdata);
> + if (ret) {
> + DRM_ERROR("failed to map hdmiphy pow control reg.\n");
> + return ret;
> + }
>
> return 0;
> }
> --
> 1.7.10.4
>
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
