Re: [PATCH 4/5] DRM: Add support for the sii902x HDMI/DVI encoder
On Tue, Jun 7, 2011 at 6:45 AM, Sascha Hauer wrote: > Signed-off-by: Sascha Hauer > --- > drivers/gpu/drm/Kconfig | 6 + > drivers/gpu/drm/i2c/Makefile | 3 + > drivers/gpu/drm/i2c/sii902x.c | 334 > + > 3 files changed, 343 insertions(+), 0 deletions(-) > create mode 100644 drivers/gpu/drm/i2c/sii902x.c > > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig > index bcd9a27..01d5444 100644 > --- a/drivers/gpu/drm/Kconfig > +++ b/drivers/gpu/drm/Kconfig > @@ -166,6 +166,12 @@ config DRM_SAVAGE > Choose this option if you have a Savage3D/4/SuperSavage/Pro/Twister > chipset. If M is selected the module will be called savage. > > +config DRM_I2C_SII902X > + tristate "sii902x" > + depends on DRM && I2C > + help > + Support for sii902x DVI/HDMI encoder chips > + > config DRM_IMX_IPUV3 > tristate "i.MX IPUv3" > depends on DRM && ARCH_MXC > diff --git a/drivers/gpu/drm/i2c/Makefile b/drivers/gpu/drm/i2c/Makefile > index 9286256..a7a8d40 100644 > --- a/drivers/gpu/drm/i2c/Makefile > +++ b/drivers/gpu/drm/i2c/Makefile > @@ -5,3 +5,6 @@ obj-$(CONFIG_DRM_I2C_CH7006) += ch7006.o > > sil164-y := sil164_drv.o > obj-$(CONFIG_DRM_I2C_SIL164) += sil164.o > + > +sii902x := sii902x_drv.o > +obj-$(CONFIG_DRM_I2C_SII902X) += sii902x.o > diff --git a/drivers/gpu/drm/i2c/sii902x.c b/drivers/gpu/drm/i2c/sii902x.c > new file mode 100644 > index 000..7928533 > --- /dev/null > +++ b/drivers/gpu/drm/i2c/sii902x.c > @@ -0,0 +1,334 @@ > +/* > + * Copyright (C) 2010 Francisco Jerez. Update the copyright? Alex > + * All Rights Reserved. > + * > + * Permission is hereby granted, free of charge, to any person obtaining > + * a copy of this software and associated documentation files (the > + * "Software"), to deal in the Software without restriction, including > + * without limitation the rights to use, copy, modify, merge, publish, > + * distribute, sublicense, and/or sell copies of the Software, and to > + * permit persons to whom the Software is furnished to do so, subject to > + * the following conditions: > + * > + * The above copyright notice and this permission notice (including the > + * next paragraph) shall be included in all copies or substantial > + * portions of the Software. > + * > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, > + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF > + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. > + * IN NO EVENT SHALL THE COPYRIGHT OWNER(S) AND/OR ITS SUPPLIERS BE > + * LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION > + * OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION > + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. > + * > + */ > + > +#include > +#include > +#include > +#include > + > +struct sii902x_encoder_params { > +}; > + > +struct sii902x_priv { > + struct sii902x_encoder_params config; > + struct i2c_client *client; > + struct drm_encoder_connector encon; > +}; > + > +#define to_sii902x(x) container_of(x, struct sii902x_priv, encon) > + > +static int sii902x_write(struct i2c_client *client, uint8_t addr, uint8_t > val) > +{ > + int ret; > + > + ret = i2c_smbus_write_byte_data(client, addr, val); > + if (ret) { > + dev_dbg(&client->dev, "%s failed with %d\n", __func__, ret); > + } > + return ret; > +} > + > +static uint8_t sii902x_read(struct i2c_client *client, uint8_t addr) > +{ > + int dat; > + > + dat = i2c_smbus_read_byte_data(client, addr); > + > + return dat; > +} > + > +static int hdmi_cap = 0; /* FIXME */ > + > +static void sii902x_poweron(struct sii902x_priv *priv) > +{ > + struct i2c_client *client = priv->client; > + > + /* Turn on DVI or HDMI */ > + if (hdmi_cap) > + sii902x_write(client, 0x1A, 0x01 | 4); > + else > + sii902x_write(client, 0x1A, 0x00); > + > + return; > +} > + > +static void sii902x_poweroff(struct sii902x_priv *priv) > +{ > + struct i2c_client *client = priv->client; > + > + /* disable tmds before changing resolution */ > + if (hdmi_cap) > + sii902x_write(client, 0x1A, 0x11); > + else > + sii902x_write(client, 0x1A, 0x10); > + > + return; > +} > + > +static int sii902x_get_modes(struct drm_encoder_connector *encon) > +{ > + struct sii902x_priv *priv = to_sii902x(encon); > + struct i2c_client *client = priv->client; > + struct i2c_adapter *adap = client->adapter; > + struct drm_connector *connector = &encon->connector; > + struct edid *edid; > + int ret; > + int old, dat, cnt = 100; > + > + old = sii902x_read(client, 0x1A); > + > + sii902x_write(client, 0x1A, old | 0x4); > + do { > + cnt--; > + msleep(
Re: [PATCH 4/5] DRM: Add support for the sii902x HDMI/DVI encoder
2011/6/7 Sascha Hauer : [...] > --- /dev/null > +++ b/drivers/gpu/drm/i2c/sii902x.c > @@ -0,0 +1,334 @@ [...] > +static int sii902x_write(struct i2c_client *client, uint8_t addr, uint8_t > val) > +{ > + int ret; > + > + ret = i2c_smbus_write_byte_data(client, addr, val); > + if (ret) { > + dev_dbg(&client->dev, "%s failed with %d\n", __func__, ret); > + } > + return ret; > +} Return value is never tested. > +static irqreturn_t sii902x_detect_handler(int irq, void *data) > +{ > + struct sii902x_priv *priv = data; > + struct i2c_client *client = priv->client; > + int dat; > + > + dat = sii902x_read(client, 0x3D); > + if (dat & 0x1) { > + /* cable connection changes */ > + if (dat & 0x4) { > + printk("plugin\n"); > + } else { > + printk("plugout\n"); > + } Missing code? > + } > + sii902x_write(client, 0x3D, dat); > + > + return IRQ_HANDLED; > +} [...] > +/* I2C driver functions */ > + > +static int > +sii902x_probe(struct i2c_client *client, const struct i2c_device_id *id) > +{ > + int dat, ret; > + struct sii902x_priv *priv; > + const char *drm_name = "imx-drm.0"; /* FIXME: pass from pdata */ > + int encon_id = 0; /* FIXME: pass from pdata */ > + > + priv = kzalloc(sizeof(*priv), GFP_KERNEL); > + if (!priv) > + return -ENOMEM; > + > + priv->client = client; > + > + /* Set 902x in hardware TPI mode on and jump out of D3 state */ > + if (sii902x_write(client, 0xc7, 0x00) < 0) { > + dev_err(&client->dev, "SII902x: cound not find device\n"); > + return -ENODEV; Leaks priv. Same on other error paths. > + } [...] > + > + > +static int sii902x_remove(struct i2c_client *client) > +{ > + struct sii902x_priv *priv; > + int ret; > + > + priv = i2c_get_clientdata(client); > + > + ret = drm_encon_unregister(&priv->encon); > + if (ret) > + return ret; Leaks priv on error. > + > + kfree(priv); > + > + return 0; > +} [...] Best Regards, Michał Mirosław ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 4/5] DRM: Add support for the sii902x HDMI/DVI encoder
On Fri, Jul 08, 2011 at 11:01:18PM +0200, Michał Mirosław wrote: > 2011/6/7 Sascha Hauer : > [...] > > --- /dev/null > > +++ b/drivers/gpu/drm/i2c/sii902x.c > > @@ -0,0 +1,334 @@ > [...] > > +static int sii902x_write(struct i2c_client *client, uint8_t addr, uint8_t > > val) > > +{ > > + � � � int ret; > > + > > + � � � ret = i2c_smbus_write_byte_data(client, addr, val); > > + � � � if (ret) { > > + � � � � � � � dev_dbg(&client->dev, "%s failed with %d\n", __func__, ret); > > + � � � } > > + � � � return ret; > > +} > > Return value is never tested. Yes, but there is not much I can do about it. This function is called from void functions most of the time, so I can't even forward the error. > > > +static irqreturn_t sii902x_detect_handler(int irq, void *data) > > +{ > > + � � � struct sii902x_priv *priv = data; > > + � � � struct i2c_client *client = priv->client; > > + � � � int dat; > > + > > + � � � dat = sii902x_read(client, 0x3D); > > + � � � if (dat & 0x1) { > > + � � � � � � � /* cable connection changes */ > > + � � � � � � � if (dat & 0x4) { > > + � � � � � � � � � � � printk("plugin\n"); > > + � � � � � � � } else { > > + � � � � � � � � � � � printk("plugout\n"); > > + � � � � � � � } > > Missing code? Yes. I will either remove interrupt support or put the missing code in. > > > + � � � } > > + � � � sii902x_write(client, 0x3D, dat); > > + > > + � � � return IRQ_HANDLED; > > +} > [...] > > +/* I2C driver functions */ > > + > > +static int > > +sii902x_probe(struct i2c_client *client, const struct i2c_device_id *id) > > +{ > > + � � � int dat, ret; > > + � � � struct sii902x_priv *priv; > > + � � � const char *drm_name = "imx-drm.0"; /* FIXME: pass from pdata */ > > + � � � int encon_id = 0; /* FIXME: pass from pdata */ > > + > > + � � � priv = kzalloc(sizeof(*priv), GFP_KERNEL); > > + � � � if (!priv) > > + � � � � � � � return -ENOMEM; > > + > > + � � � priv->client = client; > > + > > + � � � /* Set 902x in hardware TPI mode on and jump out of D3 state */ > > + � � � if (sii902x_write(client, 0xc7, 0x00) < 0) { > > + � � � � � � � dev_err(&client->dev, "SII902x: cound not find device\n"); > > + � � � � � � � return -ENODEV; > > Leaks priv. Same on other error paths. Jup, thanks for noting. There are some other bugs in the probe function, like not checking the return value of drm_encon_register(). Will fix > > > + � � � } > [...] > > > + > > + > > +static int sii902x_remove(struct i2c_client *client) > > +{ > > + � � � struct sii902x_priv *priv; > > + � � � int ret; > > + > > + � � � priv = i2c_get_clientdata(client); > > + > > + � � � ret = drm_encon_unregister(&priv->encon); > > + � � � if (ret) > > + � � � � � � � return ret; > > Leaks priv on error. and forgets to free_irq() Thanks Sascha -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 4/5] DRM: Add support for the sii902x HDMI/DVI encoder
On Tue, Jun 7, 2011 at 6:45 AM, Sascha Hauer wrote: > Signed-off-by: Sascha Hauer > --- > drivers/gpu/drm/Kconfig | 6 + > drivers/gpu/drm/i2c/Makefile | 3 + > drivers/gpu/drm/i2c/sii902x.c | 334 > + > 3 files changed, 343 insertions(+), 0 deletions(-) > create mode 100644 drivers/gpu/drm/i2c/sii902x.c > > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig > index bcd9a27..01d5444 100644 > --- a/drivers/gpu/drm/Kconfig > +++ b/drivers/gpu/drm/Kconfig > @@ -166,6 +166,12 @@ config DRM_SAVAGE > Choose this option if you have a Savage3D/4/SuperSavage/Pro/Twister > chipset. If M is selected the module will be called savage. > > +config DRM_I2C_SII902X > + tristate "sii902x" > + depends on DRM && I2C > + help > + Support for sii902x DVI/HDMI encoder chips > + > config DRM_IMX_IPUV3 > tristate "i.MX IPUv3" > depends on DRM && ARCH_MXC > diff --git a/drivers/gpu/drm/i2c/Makefile b/drivers/gpu/drm/i2c/Makefile > index 9286256..a7a8d40 100644 > --- a/drivers/gpu/drm/i2c/Makefile > +++ b/drivers/gpu/drm/i2c/Makefile > @@ -5,3 +5,6 @@ obj-$(CONFIG_DRM_I2C_CH7006) += ch7006.o > > sil164-y := sil164_drv.o > obj-$(CONFIG_DRM_I2C_SIL164) += sil164.o > + > +sii902x := sii902x_drv.o > +obj-$(CONFIG_DRM_I2C_SII902X) += sii902x.o > diff --git a/drivers/gpu/drm/i2c/sii902x.c b/drivers/gpu/drm/i2c/sii902x.c > new file mode 100644 > index 000..7928533 > --- /dev/null > +++ b/drivers/gpu/drm/i2c/sii902x.c > @@ -0,0 +1,334 @@ > +/* > + * Copyright (C) 2010 Francisco Jerez. Update the copyright? Alex > + * All Rights Reserved. > + * > + * Permission is hereby granted, free of charge, to any person obtaining > + * a copy of this software and associated documentation files (the > + * "Software"), to deal in the Software without restriction, including > + * without limitation the rights to use, copy, modify, merge, publish, > + * distribute, sublicense, and/or sell copies of the Software, and to > + * permit persons to whom the Software is furnished to do so, subject to > + * the following conditions: > + * > + * The above copyright notice and this permission notice (including the > + * next paragraph) shall be included in all copies or substantial > + * portions of the Software. > + * > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, > + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF > + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. > + * IN NO EVENT SHALL THE COPYRIGHT OWNER(S) AND/OR ITS SUPPLIERS BE > + * LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION > + * OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION > + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. > + * > + */ > + > +#include > +#include > +#include > +#include > + > +struct sii902x_encoder_params { > +}; > + > +struct sii902x_priv { > + struct sii902x_encoder_params config; > + struct i2c_client *client; > + struct drm_encoder_connector encon; > +}; > + > +#define to_sii902x(x) container_of(x, struct sii902x_priv, encon) > + > +static int sii902x_write(struct i2c_client *client, uint8_t addr, uint8_t > val) > +{ > + int ret; > + > + ret = i2c_smbus_write_byte_data(client, addr, val); > + if (ret) { > + dev_dbg(&client->dev, "%s failed with %d\n", __func__, ret); > + } > + return ret; > +} > + > +static uint8_t sii902x_read(struct i2c_client *client, uint8_t addr) > +{ > + int dat; > + > + dat = i2c_smbus_read_byte_data(client, addr); > + > + return dat; > +} > + > +static int hdmi_cap = 0; /* FIXME */ > + > +static void sii902x_poweron(struct sii902x_priv *priv) > +{ > + struct i2c_client *client = priv->client; > + > + /* Turn on DVI or HDMI */ > + if (hdmi_cap) > + sii902x_write(client, 0x1A, 0x01 | 4); > + else > + sii902x_write(client, 0x1A, 0x00); > + > + return; > +} > + > +static void sii902x_poweroff(struct sii902x_priv *priv) > +{ > + struct i2c_client *client = priv->client; > + > + /* disable tmds before changing resolution */ > + if (hdmi_cap) > + sii902x_write(client, 0x1A, 0x11); > + else > + sii902x_write(client, 0x1A, 0x10); > + > + return; > +} > + > +static int sii902x_get_modes(struct drm_encoder_connector *encon) > +{ > + struct sii902x_priv *priv = to_sii902x(encon); > + struct i2c_client *client = priv->client; > + struct i2c_adapter *adap = client->adapter; > + struct drm_connector *connector = &encon->connector; > + struct edid *edid; > + int ret; > + int old, dat, cnt = 100; > + > + old = sii902x_read(client, 0x1A); > + > + sii902x_write(client, 0x1A, old | 0x4); > + do { > + cnt--; > + msleep(
Re: [PATCH 4/5] DRM: Add support for the sii902x HDMI/DVI encoder
2011/6/7 Sascha Hauer : [...] > --- /dev/null > +++ b/drivers/gpu/drm/i2c/sii902x.c > @@ -0,0 +1,334 @@ [...] > +static int sii902x_write(struct i2c_client *client, uint8_t addr, uint8_t > val) > +{ > + int ret; > + > + ret = i2c_smbus_write_byte_data(client, addr, val); > + if (ret) { > + dev_dbg(&client->dev, "%s failed with %d\n", __func__, ret); > + } > + return ret; > +} Return value is never tested. > +static irqreturn_t sii902x_detect_handler(int irq, void *data) > +{ > + struct sii902x_priv *priv = data; > + struct i2c_client *client = priv->client; > + int dat; > + > + dat = sii902x_read(client, 0x3D); > + if (dat & 0x1) { > + /* cable connection changes */ > + if (dat & 0x4) { > + printk("plugin\n"); > + } else { > + printk("plugout\n"); > + } Missing code? > + } > + sii902x_write(client, 0x3D, dat); > + > + return IRQ_HANDLED; > +} [...] > +/* I2C driver functions */ > + > +static int > +sii902x_probe(struct i2c_client *client, const struct i2c_device_id *id) > +{ > + int dat, ret; > + struct sii902x_priv *priv; > + const char *drm_name = "imx-drm.0"; /* FIXME: pass from pdata */ > + int encon_id = 0; /* FIXME: pass from pdata */ > + > + priv = kzalloc(sizeof(*priv), GFP_KERNEL); > + if (!priv) > + return -ENOMEM; > + > + priv->client = client; > + > + /* Set 902x in hardware TPI mode on and jump out of D3 state */ > + if (sii902x_write(client, 0xc7, 0x00) < 0) { > + dev_err(&client->dev, "SII902x: cound not find device\n"); > + return -ENODEV; Leaks priv. Same on other error paths. > + } [...] > + > + > +static int sii902x_remove(struct i2c_client *client) > +{ > + struct sii902x_priv *priv; > + int ret; > + > + priv = i2c_get_clientdata(client); > + > + ret = drm_encon_unregister(&priv->encon); > + if (ret) > + return ret; Leaks priv on error. > + > + kfree(priv); > + > + return 0; > +} [...] Best Regards, Michał Mirosław ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 4/5] DRM: Add support for the sii902x HDMI/DVI encoder
On Fri, Jul 08, 2011 at 11:01:18PM +0200, Michał Mirosław wrote: > 2011/6/7 Sascha Hauer : > [...] > > --- /dev/null > > +++ b/drivers/gpu/drm/i2c/sii902x.c > > @@ -0,0 +1,334 @@ > [...] > > +static int sii902x_write(struct i2c_client *client, uint8_t addr, uint8_t > > val) > > +{ > > + � � � int ret; > > + > > + � � � ret = i2c_smbus_write_byte_data(client, addr, val); > > + � � � if (ret) { > > + � � � � � � � dev_dbg(&client->dev, "%s failed with %d\n", __func__, ret); > > + � � � } > > + � � � return ret; > > +} > > Return value is never tested. Yes, but there is not much I can do about it. This function is called from void functions most of the time, so I can't even forward the error. > > > +static irqreturn_t sii902x_detect_handler(int irq, void *data) > > +{ > > + � � � struct sii902x_priv *priv = data; > > + � � � struct i2c_client *client = priv->client; > > + � � � int dat; > > + > > + � � � dat = sii902x_read(client, 0x3D); > > + � � � if (dat & 0x1) { > > + � � � � � � � /* cable connection changes */ > > + � � � � � � � if (dat & 0x4) { > > + � � � � � � � � � � � printk("plugin\n"); > > + � � � � � � � } else { > > + � � � � � � � � � � � printk("plugout\n"); > > + � � � � � � � } > > Missing code? Yes. I will either remove interrupt support or put the missing code in. > > > + � � � } > > + � � � sii902x_write(client, 0x3D, dat); > > + > > + � � � return IRQ_HANDLED; > > +} > [...] > > +/* I2C driver functions */ > > + > > +static int > > +sii902x_probe(struct i2c_client *client, const struct i2c_device_id *id) > > +{ > > + � � � int dat, ret; > > + � � � struct sii902x_priv *priv; > > + � � � const char *drm_name = "imx-drm.0"; /* FIXME: pass from pdata */ > > + � � � int encon_id = 0; /* FIXME: pass from pdata */ > > + > > + � � � priv = kzalloc(sizeof(*priv), GFP_KERNEL); > > + � � � if (!priv) > > + � � � � � � � return -ENOMEM; > > + > > + � � � priv->client = client; > > + > > + � � � /* Set 902x in hardware TPI mode on and jump out of D3 state */ > > + � � � if (sii902x_write(client, 0xc7, 0x00) < 0) { > > + � � � � � � � dev_err(&client->dev, "SII902x: cound not find device\n"); > > + � � � � � � � return -ENODEV; > > Leaks priv. Same on other error paths. Jup, thanks for noting. There are some other bugs in the probe function, like not checking the return value of drm_encon_register(). Will fix > > > + � � � } > [...] > > > + > > + > > +static int sii902x_remove(struct i2c_client *client) > > +{ > > + � � � struct sii902x_priv *priv; > > + � � � int ret; > > + > > + � � � priv = i2c_get_clientdata(client); > > + > > + � � � ret = drm_encon_unregister(&priv->encon); > > + � � � if (ret) > > + � � � � � � � return ret; > > Leaks priv on error. and forgets to free_irq() Thanks Sascha -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 4/5] DRM: Add support for the sii902x HDMI/DVI encoder
On Tue, Jun 7, 2011 at 6:45 AM, Sascha Hauer wrote: > Signed-off-by: Sascha Hauer > --- > drivers/gpu/drm/Kconfig | 6 + > drivers/gpu/drm/i2c/Makefile | 3 + > drivers/gpu/drm/i2c/sii902x.c | 334 > + > 3 files changed, 343 insertions(+), 0 deletions(-) > create mode 100644 drivers/gpu/drm/i2c/sii902x.c > > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig > index bcd9a27..01d5444 100644 > --- a/drivers/gpu/drm/Kconfig > +++ b/drivers/gpu/drm/Kconfig > @@ -166,6 +166,12 @@ config DRM_SAVAGE > Choose this option if you have a Savage3D/4/SuperSavage/Pro/Twister > chipset. If M is selected the module will be called savage. > > +config DRM_I2C_SII902X > + tristate "sii902x" > + depends on DRM && I2C > + help > + Support for sii902x DVI/HDMI encoder chips > + > config DRM_IMX_IPUV3 > tristate "i.MX IPUv3" > depends on DRM && ARCH_MXC > diff --git a/drivers/gpu/drm/i2c/Makefile b/drivers/gpu/drm/i2c/Makefile > index 9286256..a7a8d40 100644 > --- a/drivers/gpu/drm/i2c/Makefile > +++ b/drivers/gpu/drm/i2c/Makefile > @@ -5,3 +5,6 @@ obj-$(CONFIG_DRM_I2C_CH7006) += ch7006.o > > sil164-y := sil164_drv.o > obj-$(CONFIG_DRM_I2C_SIL164) += sil164.o > + > +sii902x := sii902x_drv.o > +obj-$(CONFIG_DRM_I2C_SII902X) += sii902x.o > diff --git a/drivers/gpu/drm/i2c/sii902x.c b/drivers/gpu/drm/i2c/sii902x.c > new file mode 100644 > index 000..7928533 > --- /dev/null > +++ b/drivers/gpu/drm/i2c/sii902x.c > @@ -0,0 +1,334 @@ > +/* > + * Copyright (C) 2010 Francisco Jerez. Update the copyright? Alex > + * All Rights Reserved. > + * > + * Permission is hereby granted, free of charge, to any person obtaining > + * a copy of this software and associated documentation files (the > + * "Software"), to deal in the Software without restriction, including > + * without limitation the rights to use, copy, modify, merge, publish, > + * distribute, sublicense, and/or sell copies of the Software, and to > + * permit persons to whom the Software is furnished to do so, subject to > + * the following conditions: > + * > + * The above copyright notice and this permission notice (including the > + * next paragraph) shall be included in all copies or substantial > + * portions of the Software. > + * > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, > + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF > + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. > + * IN NO EVENT SHALL THE COPYRIGHT OWNER(S) AND/OR ITS SUPPLIERS BE > + * LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION > + * OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION > + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. > + * > + */ > + > +#include > +#include > +#include > +#include > + > +struct sii902x_encoder_params { > +}; > + > +struct sii902x_priv { > + struct sii902x_encoder_params config; > + struct i2c_client *client; > + struct drm_encoder_connector encon; > +}; > + > +#define to_sii902x(x) container_of(x, struct sii902x_priv, encon) > + > +static int sii902x_write(struct i2c_client *client, uint8_t addr, uint8_t > val) > +{ > + int ret; > + > + ret = i2c_smbus_write_byte_data(client, addr, val); > + if (ret) { > + dev_dbg(&client->dev, "%s failed with %d\n", __func__, ret); > + } > + return ret; > +} > + > +static uint8_t sii902x_read(struct i2c_client *client, uint8_t addr) > +{ > + int dat; > + > + dat = i2c_smbus_read_byte_data(client, addr); > + > + return dat; > +} > + > +static int hdmi_cap = 0; /* FIXME */ > + > +static void sii902x_poweron(struct sii902x_priv *priv) > +{ > + struct i2c_client *client = priv->client; > + > + /* Turn on DVI or HDMI */ > + if (hdmi_cap) > + sii902x_write(client, 0x1A, 0x01 | 4); > + else > + sii902x_write(client, 0x1A, 0x00); > + > + return; > +} > + > +static void sii902x_poweroff(struct sii902x_priv *priv) > +{ > + struct i2c_client *client = priv->client; > + > + /* disable tmds before changing resolution */ > + if (hdmi_cap) > + sii902x_write(client, 0x1A, 0x11); > + else > + sii902x_write(client, 0x1A, 0x10); > + > + return; > +} > + > +static int sii902x_get_modes(struct drm_encoder_connector *encon) > +{ > + struct sii902x_priv *priv = to_sii902x(encon); > + struct i2c_client *client = priv->client; > + struct i2c_adapter *adap = client->adapter; > + struct drm_connector *connector = &encon->connector; > + struct edid *edid; > + int ret; > + int old, dat, cnt = 100; > + > + old = sii902x_read(client, 0x1A); > + > + sii902x_write(client, 0x1A, old | 0x4); > + do { > + cnt--; > + msleep(
Re: [PATCH 4/5] DRM: Add support for the sii902x HDMI/DVI encoder
2011/6/7 Sascha Hauer : [...] > --- /dev/null > +++ b/drivers/gpu/drm/i2c/sii902x.c > @@ -0,0 +1,334 @@ [...] > +static int sii902x_write(struct i2c_client *client, uint8_t addr, uint8_t > val) > +{ > + int ret; > + > + ret = i2c_smbus_write_byte_data(client, addr, val); > + if (ret) { > + dev_dbg(&client->dev, "%s failed with %d\n", __func__, ret); > + } > + return ret; > +} Return value is never tested. > +static irqreturn_t sii902x_detect_handler(int irq, void *data) > +{ > + struct sii902x_priv *priv = data; > + struct i2c_client *client = priv->client; > + int dat; > + > + dat = sii902x_read(client, 0x3D); > + if (dat & 0x1) { > + /* cable connection changes */ > + if (dat & 0x4) { > + printk("plugin\n"); > + } else { > + printk("plugout\n"); > + } Missing code? > + } > + sii902x_write(client, 0x3D, dat); > + > + return IRQ_HANDLED; > +} [...] > +/* I2C driver functions */ > + > +static int > +sii902x_probe(struct i2c_client *client, const struct i2c_device_id *id) > +{ > + int dat, ret; > + struct sii902x_priv *priv; > + const char *drm_name = "imx-drm.0"; /* FIXME: pass from pdata */ > + int encon_id = 0; /* FIXME: pass from pdata */ > + > + priv = kzalloc(sizeof(*priv), GFP_KERNEL); > + if (!priv) > + return -ENOMEM; > + > + priv->client = client; > + > + /* Set 902x in hardware TPI mode on and jump out of D3 state */ > + if (sii902x_write(client, 0xc7, 0x00) < 0) { > + dev_err(&client->dev, "SII902x: cound not find device\n"); > + return -ENODEV; Leaks priv. Same on other error paths. > + } [...] > + > + > +static int sii902x_remove(struct i2c_client *client) > +{ > + struct sii902x_priv *priv; > + int ret; > + > + priv = i2c_get_clientdata(client); > + > + ret = drm_encon_unregister(&priv->encon); > + if (ret) > + return ret; Leaks priv on error. > + > + kfree(priv); > + > + return 0; > +} [...] Best Regards, Michał Mirosław ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 4/5] DRM: Add support for the sii902x HDMI/DVI encoder
On Fri, Jul 08, 2011 at 11:01:18PM +0200, Michał Mirosław wrote: > 2011/6/7 Sascha Hauer : > [...] > > --- /dev/null > > +++ b/drivers/gpu/drm/i2c/sii902x.c > > @@ -0,0 +1,334 @@ > [...] > > +static int sii902x_write(struct i2c_client *client, uint8_t addr, uint8_t > > val) > > +{ > > + � � � int ret; > > + > > + � � � ret = i2c_smbus_write_byte_data(client, addr, val); > > + � � � if (ret) { > > + � � � � � � � dev_dbg(&client->dev, "%s failed with %d\n", __func__, ret); > > + � � � } > > + � � � return ret; > > +} > > Return value is never tested. Yes, but there is not much I can do about it. This function is called from void functions most of the time, so I can't even forward the error. > > > +static irqreturn_t sii902x_detect_handler(int irq, void *data) > > +{ > > + � � � struct sii902x_priv *priv = data; > > + � � � struct i2c_client *client = priv->client; > > + � � � int dat; > > + > > + � � � dat = sii902x_read(client, 0x3D); > > + � � � if (dat & 0x1) { > > + � � � � � � � /* cable connection changes */ > > + � � � � � � � if (dat & 0x4) { > > + � � � � � � � � � � � printk("plugin\n"); > > + � � � � � � � } else { > > + � � � � � � � � � � � printk("plugout\n"); > > + � � � � � � � } > > Missing code? Yes. I will either remove interrupt support or put the missing code in. > > > + � � � } > > + � � � sii902x_write(client, 0x3D, dat); > > + > > + � � � return IRQ_HANDLED; > > +} > [...] > > +/* I2C driver functions */ > > + > > +static int > > +sii902x_probe(struct i2c_client *client, const struct i2c_device_id *id) > > +{ > > + � � � int dat, ret; > > + � � � struct sii902x_priv *priv; > > + � � � const char *drm_name = "imx-drm.0"; /* FIXME: pass from pdata */ > > + � � � int encon_id = 0; /* FIXME: pass from pdata */ > > + > > + � � � priv = kzalloc(sizeof(*priv), GFP_KERNEL); > > + � � � if (!priv) > > + � � � � � � � return -ENOMEM; > > + > > + � � � priv->client = client; > > + > > + � � � /* Set 902x in hardware TPI mode on and jump out of D3 state */ > > + � � � if (sii902x_write(client, 0xc7, 0x00) < 0) { > > + � � � � � � � dev_err(&client->dev, "SII902x: cound not find device\n"); > > + � � � � � � � return -ENODEV; > > Leaks priv. Same on other error paths. Jup, thanks for noting. There are some other bugs in the probe function, like not checking the return value of drm_encon_register(). Will fix > > > + � � � } > [...] > > > + > > + > > +static int sii902x_remove(struct i2c_client *client) > > +{ > > + � � � struct sii902x_priv *priv; > > + � � � int ret; > > + > > + � � � priv = i2c_get_clientdata(client); > > + > > + � � � ret = drm_encon_unregister(&priv->encon); > > + � � � if (ret) > > + � � � � � � � return ret; > > Leaks priv on error. and forgets to free_irq() Thanks Sascha -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 4/5] DRM: Add support for the sii902x HDMI/DVI encoder
On Tue, Jun 7, 2011 at 6:45 AM, Sascha Hauer wrote: > Signed-off-by: Sascha Hauer > --- > drivers/gpu/drm/Kconfig | 6 + > drivers/gpu/drm/i2c/Makefile | 3 + > drivers/gpu/drm/i2c/sii902x.c | 334 > + > 3 files changed, 343 insertions(+), 0 deletions(-) > create mode 100644 drivers/gpu/drm/i2c/sii902x.c > > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig > index bcd9a27..01d5444 100644 > --- a/drivers/gpu/drm/Kconfig > +++ b/drivers/gpu/drm/Kconfig > @@ -166,6 +166,12 @@ config DRM_SAVAGE > Choose this option if you have a Savage3D/4/SuperSavage/Pro/Twister > chipset. If M is selected the module will be called savage. > > +config DRM_I2C_SII902X > + tristate "sii902x" > + depends on DRM && I2C > + help > + Support for sii902x DVI/HDMI encoder chips > + > config DRM_IMX_IPUV3 > tristate "i.MX IPUv3" > depends on DRM && ARCH_MXC > diff --git a/drivers/gpu/drm/i2c/Makefile b/drivers/gpu/drm/i2c/Makefile > index 9286256..a7a8d40 100644 > --- a/drivers/gpu/drm/i2c/Makefile > +++ b/drivers/gpu/drm/i2c/Makefile > @@ -5,3 +5,6 @@ obj-$(CONFIG_DRM_I2C_CH7006) += ch7006.o > > sil164-y := sil164_drv.o > obj-$(CONFIG_DRM_I2C_SIL164) += sil164.o > + > +sii902x := sii902x_drv.o > +obj-$(CONFIG_DRM_I2C_SII902X) += sii902x.o > diff --git a/drivers/gpu/drm/i2c/sii902x.c b/drivers/gpu/drm/i2c/sii902x.c > new file mode 100644 > index 000..7928533 > --- /dev/null > +++ b/drivers/gpu/drm/i2c/sii902x.c > @@ -0,0 +1,334 @@ > +/* > + * Copyright (C) 2010 Francisco Jerez. Update the copyright? Alex > + * All Rights Reserved. > + * > + * Permission is hereby granted, free of charge, to any person obtaining > + * a copy of this software and associated documentation files (the > + * "Software"), to deal in the Software without restriction, including > + * without limitation the rights to use, copy, modify, merge, publish, > + * distribute, sublicense, and/or sell copies of the Software, and to > + * permit persons to whom the Software is furnished to do so, subject to > + * the following conditions: > + * > + * The above copyright notice and this permission notice (including the > + * next paragraph) shall be included in all copies or substantial > + * portions of the Software. > + * > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, > + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF > + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. > + * IN NO EVENT SHALL THE COPYRIGHT OWNER(S) AND/OR ITS SUPPLIERS BE > + * LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION > + * OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION > + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. > + * > + */ > + > +#include > +#include > +#include > +#include > + > +struct sii902x_encoder_params { > +}; > + > +struct sii902x_priv { > + struct sii902x_encoder_params config; > + struct i2c_client *client; > + struct drm_encoder_connector encon; > +}; > + > +#define to_sii902x(x) container_of(x, struct sii902x_priv, encon) > + > +static int sii902x_write(struct i2c_client *client, uint8_t addr, uint8_t > val) > +{ > + int ret; > + > + ret = i2c_smbus_write_byte_data(client, addr, val); > + if (ret) { > + dev_dbg(&client->dev, "%s failed with %d\n", __func__, ret); > + } > + return ret; > +} > + > +static uint8_t sii902x_read(struct i2c_client *client, uint8_t addr) > +{ > + int dat; > + > + dat = i2c_smbus_read_byte_data(client, addr); > + > + return dat; > +} > + > +static int hdmi_cap = 0; /* FIXME */ > + > +static void sii902x_poweron(struct sii902x_priv *priv) > +{ > + struct i2c_client *client = priv->client; > + > + /* Turn on DVI or HDMI */ > + if (hdmi_cap) > + sii902x_write(client, 0x1A, 0x01 | 4); > + else > + sii902x_write(client, 0x1A, 0x00); > + > + return; > +} > + > +static void sii902x_poweroff(struct sii902x_priv *priv) > +{ > + struct i2c_client *client = priv->client; > + > + /* disable tmds before changing resolution */ > + if (hdmi_cap) > + sii902x_write(client, 0x1A, 0x11); > + else > + sii902x_write(client, 0x1A, 0x10); > + > + return; > +} > + > +static int sii902x_get_modes(struct drm_encoder_connector *encon) > +{ > + struct sii902x_priv *priv = to_sii902x(encon); > + struct i2c_client *client = priv->client; > + struct i2c_adapter *adap = client->adapter; > + struct drm_connector *connector = &encon->connector; > + struct edid *edid; > + int ret; > + int old, dat, cnt = 100; > + > + old = sii902x_read(client, 0x1A); > + > + sii902x_write(client, 0x1A, old | 0x4); > + do { > + cnt--; > + msleep(
Re: [PATCH 4/5] DRM: Add support for the sii902x HDMI/DVI encoder
2011/6/7 Sascha Hauer : [...] > --- /dev/null > +++ b/drivers/gpu/drm/i2c/sii902x.c > @@ -0,0 +1,334 @@ [...] > +static int sii902x_write(struct i2c_client *client, uint8_t addr, uint8_t > val) > +{ > + int ret; > + > + ret = i2c_smbus_write_byte_data(client, addr, val); > + if (ret) { > + dev_dbg(&client->dev, "%s failed with %d\n", __func__, ret); > + } > + return ret; > +} Return value is never tested. > +static irqreturn_t sii902x_detect_handler(int irq, void *data) > +{ > + struct sii902x_priv *priv = data; > + struct i2c_client *client = priv->client; > + int dat; > + > + dat = sii902x_read(client, 0x3D); > + if (dat & 0x1) { > + /* cable connection changes */ > + if (dat & 0x4) { > + printk("plugin\n"); > + } else { > + printk("plugout\n"); > + } Missing code? > + } > + sii902x_write(client, 0x3D, dat); > + > + return IRQ_HANDLED; > +} [...] > +/* I2C driver functions */ > + > +static int > +sii902x_probe(struct i2c_client *client, const struct i2c_device_id *id) > +{ > + int dat, ret; > + struct sii902x_priv *priv; > + const char *drm_name = "imx-drm.0"; /* FIXME: pass from pdata */ > + int encon_id = 0; /* FIXME: pass from pdata */ > + > + priv = kzalloc(sizeof(*priv), GFP_KERNEL); > + if (!priv) > + return -ENOMEM; > + > + priv->client = client; > + > + /* Set 902x in hardware TPI mode on and jump out of D3 state */ > + if (sii902x_write(client, 0xc7, 0x00) < 0) { > + dev_err(&client->dev, "SII902x: cound not find device\n"); > + return -ENODEV; Leaks priv. Same on other error paths. > + } [...] > + > + > +static int sii902x_remove(struct i2c_client *client) > +{ > + struct sii902x_priv *priv; > + int ret; > + > + priv = i2c_get_clientdata(client); > + > + ret = drm_encon_unregister(&priv->encon); > + if (ret) > + return ret; Leaks priv on error. > + > + kfree(priv); > + > + return 0; > +} [...] Best Regards, Michał Mirosław ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 4/5] DRM: Add support for the sii902x HDMI/DVI encoder
On Fri, Jul 08, 2011 at 11:01:18PM +0200, Michał Mirosław wrote: > 2011/6/7 Sascha Hauer : > [...] > > --- /dev/null > > +++ b/drivers/gpu/drm/i2c/sii902x.c > > @@ -0,0 +1,334 @@ > [...] > > +static int sii902x_write(struct i2c_client *client, uint8_t addr, uint8_t > > val) > > +{ > > + � � � int ret; > > + > > + � � � ret = i2c_smbus_write_byte_data(client, addr, val); > > + � � � if (ret) { > > + � � � � � � � dev_dbg(&client->dev, "%s failed with %d\n", __func__, ret); > > + � � � } > > + � � � return ret; > > +} > > Return value is never tested. Yes, but there is not much I can do about it. This function is called from void functions most of the time, so I can't even forward the error. > > > +static irqreturn_t sii902x_detect_handler(int irq, void *data) > > +{ > > + � � � struct sii902x_priv *priv = data; > > + � � � struct i2c_client *client = priv->client; > > + � � � int dat; > > + > > + � � � dat = sii902x_read(client, 0x3D); > > + � � � if (dat & 0x1) { > > + � � � � � � � /* cable connection changes */ > > + � � � � � � � if (dat & 0x4) { > > + � � � � � � � � � � � printk("plugin\n"); > > + � � � � � � � } else { > > + � � � � � � � � � � � printk("plugout\n"); > > + � � � � � � � } > > Missing code? Yes. I will either remove interrupt support or put the missing code in. > > > + � � � } > > + � � � sii902x_write(client, 0x3D, dat); > > + > > + � � � return IRQ_HANDLED; > > +} > [...] > > +/* I2C driver functions */ > > + > > +static int > > +sii902x_probe(struct i2c_client *client, const struct i2c_device_id *id) > > +{ > > + � � � int dat, ret; > > + � � � struct sii902x_priv *priv; > > + � � � const char *drm_name = "imx-drm.0"; /* FIXME: pass from pdata */ > > + � � � int encon_id = 0; /* FIXME: pass from pdata */ > > + > > + � � � priv = kzalloc(sizeof(*priv), GFP_KERNEL); > > + � � � if (!priv) > > + � � � � � � � return -ENOMEM; > > + > > + � � � priv->client = client; > > + > > + � � � /* Set 902x in hardware TPI mode on and jump out of D3 state */ > > + � � � if (sii902x_write(client, 0xc7, 0x00) < 0) { > > + � � � � � � � dev_err(&client->dev, "SII902x: cound not find device\n"); > > + � � � � � � � return -ENODEV; > > Leaks priv. Same on other error paths. Jup, thanks for noting. There are some other bugs in the probe function, like not checking the return value of drm_encon_register(). Will fix > > > + � � � } > [...] > > > + > > + > > +static int sii902x_remove(struct i2c_client *client) > > +{ > > + � � � struct sii902x_priv *priv; > > + � � � int ret; > > + > > + � � � priv = i2c_get_clientdata(client); > > + > > + � � � ret = drm_encon_unregister(&priv->encon); > > + � � � if (ret) > > + � � � � � � � return ret; > > Leaks priv on error. and forgets to free_irq() Thanks Sascha -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 4/5] DRM: Add support for the sii902x HDMI/DVI encoder
On Tue, Jun 7, 2011 at 6:45 AM, Sascha Hauer wrote: > Signed-off-by: Sascha Hauer > --- > drivers/gpu/drm/Kconfig | 6 + > drivers/gpu/drm/i2c/Makefile | 3 + > drivers/gpu/drm/i2c/sii902x.c | 334 > + > 3 files changed, 343 insertions(+), 0 deletions(-) > create mode 100644 drivers/gpu/drm/i2c/sii902x.c > > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig > index bcd9a27..01d5444 100644 > --- a/drivers/gpu/drm/Kconfig > +++ b/drivers/gpu/drm/Kconfig > @@ -166,6 +166,12 @@ config DRM_SAVAGE > Choose this option if you have a Savage3D/4/SuperSavage/Pro/Twister > chipset. If M is selected the module will be called savage. > > +config DRM_I2C_SII902X > + tristate "sii902x" > + depends on DRM && I2C > + help > + Support for sii902x DVI/HDMI encoder chips > + > config DRM_IMX_IPUV3 > tristate "i.MX IPUv3" > depends on DRM && ARCH_MXC > diff --git a/drivers/gpu/drm/i2c/Makefile b/drivers/gpu/drm/i2c/Makefile > index 9286256..a7a8d40 100644 > --- a/drivers/gpu/drm/i2c/Makefile > +++ b/drivers/gpu/drm/i2c/Makefile > @@ -5,3 +5,6 @@ obj-$(CONFIG_DRM_I2C_CH7006) += ch7006.o > > sil164-y := sil164_drv.o > obj-$(CONFIG_DRM_I2C_SIL164) += sil164.o > + > +sii902x := sii902x_drv.o > +obj-$(CONFIG_DRM_I2C_SII902X) += sii902x.o > diff --git a/drivers/gpu/drm/i2c/sii902x.c b/drivers/gpu/drm/i2c/sii902x.c > new file mode 100644 > index 000..7928533 > --- /dev/null > +++ b/drivers/gpu/drm/i2c/sii902x.c > @@ -0,0 +1,334 @@ > +/* > + * Copyright (C) 2010 Francisco Jerez. Update the copyright? Alex > + * All Rights Reserved. > + * > + * Permission is hereby granted, free of charge, to any person obtaining > + * a copy of this software and associated documentation files (the > + * "Software"), to deal in the Software without restriction, including > + * without limitation the rights to use, copy, modify, merge, publish, > + * distribute, sublicense, and/or sell copies of the Software, and to > + * permit persons to whom the Software is furnished to do so, subject to > + * the following conditions: > + * > + * The above copyright notice and this permission notice (including the > + * next paragraph) shall be included in all copies or substantial > + * portions of the Software. > + * > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, > + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF > + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. > + * IN NO EVENT SHALL THE COPYRIGHT OWNER(S) AND/OR ITS SUPPLIERS BE > + * LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION > + * OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION > + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. > + * > + */ > + > +#include > +#include > +#include > +#include > + > +struct sii902x_encoder_params { > +}; > + > +struct sii902x_priv { > + struct sii902x_encoder_params config; > + struct i2c_client *client; > + struct drm_encoder_connector encon; > +}; > + > +#define to_sii902x(x) container_of(x, struct sii902x_priv, encon) > + > +static int sii902x_write(struct i2c_client *client, uint8_t addr, uint8_t > val) > +{ > + int ret; > + > + ret = i2c_smbus_write_byte_data(client, addr, val); > + if (ret) { > + dev_dbg(&client->dev, "%s failed with %d\n", __func__, ret); > + } > + return ret; > +} > + > +static uint8_t sii902x_read(struct i2c_client *client, uint8_t addr) > +{ > + int dat; > + > + dat = i2c_smbus_read_byte_data(client, addr); > + > + return dat; > +} > + > +static int hdmi_cap = 0; /* FIXME */ > + > +static void sii902x_poweron(struct sii902x_priv *priv) > +{ > + struct i2c_client *client = priv->client; > + > + /* Turn on DVI or HDMI */ > + if (hdmi_cap) > + sii902x_write(client, 0x1A, 0x01 | 4); > + else > + sii902x_write(client, 0x1A, 0x00); > + > + return; > +} > + > +static void sii902x_poweroff(struct sii902x_priv *priv) > +{ > + struct i2c_client *client = priv->client; > + > + /* disable tmds before changing resolution */ > + if (hdmi_cap) > + sii902x_write(client, 0x1A, 0x11); > + else > + sii902x_write(client, 0x1A, 0x10); > + > + return; > +} > + > +static int sii902x_get_modes(struct drm_encoder_connector *encon) > +{ > + struct sii902x_priv *priv = to_sii902x(encon); > + struct i2c_client *client = priv->client; > + struct i2c_adapter *adap = client->adapter; > + struct drm_connector *connector = &encon->connector; > + struct edid *edid; > + int ret; > + int old, dat, cnt = 100; > + > + old = sii902x_read(client, 0x1A); > + > + sii902x_write(client, 0x1A, old | 0x4); > + do { > + cnt--; > + msleep(
Re: [PATCH 4/5] DRM: Add support for the sii902x HDMI/DVI encoder
2011/6/7 Sascha Hauer : [...] > --- /dev/null > +++ b/drivers/gpu/drm/i2c/sii902x.c > @@ -0,0 +1,334 @@ [...] > +static int sii902x_write(struct i2c_client *client, uint8_t addr, uint8_t > val) > +{ > + int ret; > + > + ret = i2c_smbus_write_byte_data(client, addr, val); > + if (ret) { > + dev_dbg(&client->dev, "%s failed with %d\n", __func__, ret); > + } > + return ret; > +} Return value is never tested. > +static irqreturn_t sii902x_detect_handler(int irq, void *data) > +{ > + struct sii902x_priv *priv = data; > + struct i2c_client *client = priv->client; > + int dat; > + > + dat = sii902x_read(client, 0x3D); > + if (dat & 0x1) { > + /* cable connection changes */ > + if (dat & 0x4) { > + printk("plugin\n"); > + } else { > + printk("plugout\n"); > + } Missing code? > + } > + sii902x_write(client, 0x3D, dat); > + > + return IRQ_HANDLED; > +} [...] > +/* I2C driver functions */ > + > +static int > +sii902x_probe(struct i2c_client *client, const struct i2c_device_id *id) > +{ > + int dat, ret; > + struct sii902x_priv *priv; > + const char *drm_name = "imx-drm.0"; /* FIXME: pass from pdata */ > + int encon_id = 0; /* FIXME: pass from pdata */ > + > + priv = kzalloc(sizeof(*priv), GFP_KERNEL); > + if (!priv) > + return -ENOMEM; > + > + priv->client = client; > + > + /* Set 902x in hardware TPI mode on and jump out of D3 state */ > + if (sii902x_write(client, 0xc7, 0x00) < 0) { > + dev_err(&client->dev, "SII902x: cound not find device\n"); > + return -ENODEV; Leaks priv. Same on other error paths. > + } [...] > + > + > +static int sii902x_remove(struct i2c_client *client) > +{ > + struct sii902x_priv *priv; > + int ret; > + > + priv = i2c_get_clientdata(client); > + > + ret = drm_encon_unregister(&priv->encon); > + if (ret) > + return ret; Leaks priv on error. > + > + kfree(priv); > + > + return 0; > +} [...] Best Regards, Michał Mirosław ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 4/5] DRM: Add support for the sii902x HDMI/DVI encoder
On Fri, Jul 08, 2011 at 11:01:18PM +0200, Michał Mirosław wrote: > 2011/6/7 Sascha Hauer : > [...] > > --- /dev/null > > +++ b/drivers/gpu/drm/i2c/sii902x.c > > @@ -0,0 +1,334 @@ > [...] > > +static int sii902x_write(struct i2c_client *client, uint8_t addr, uint8_t > > val) > > +{ > > + � � � int ret; > > + > > + � � � ret = i2c_smbus_write_byte_data(client, addr, val); > > + � � � if (ret) { > > + � � � � � � � dev_dbg(&client->dev, "%s failed with %d\n", __func__, ret); > > + � � � } > > + � � � return ret; > > +} > > Return value is never tested. Yes, but there is not much I can do about it. This function is called from void functions most of the time, so I can't even forward the error. > > > +static irqreturn_t sii902x_detect_handler(int irq, void *data) > > +{ > > + � � � struct sii902x_priv *priv = data; > > + � � � struct i2c_client *client = priv->client; > > + � � � int dat; > > + > > + � � � dat = sii902x_read(client, 0x3D); > > + � � � if (dat & 0x1) { > > + � � � � � � � /* cable connection changes */ > > + � � � � � � � if (dat & 0x4) { > > + � � � � � � � � � � � printk("plugin\n"); > > + � � � � � � � } else { > > + � � � � � � � � � � � printk("plugout\n"); > > + � � � � � � � } > > Missing code? Yes. I will either remove interrupt support or put the missing code in. > > > + � � � } > > + � � � sii902x_write(client, 0x3D, dat); > > + > > + � � � return IRQ_HANDLED; > > +} > [...] > > +/* I2C driver functions */ > > + > > +static int > > +sii902x_probe(struct i2c_client *client, const struct i2c_device_id *id) > > +{ > > + � � � int dat, ret; > > + � � � struct sii902x_priv *priv; > > + � � � const char *drm_name = "imx-drm.0"; /* FIXME: pass from pdata */ > > + � � � int encon_id = 0; /* FIXME: pass from pdata */ > > + > > + � � � priv = kzalloc(sizeof(*priv), GFP_KERNEL); > > + � � � if (!priv) > > + � � � � � � � return -ENOMEM; > > + > > + � � � priv->client = client; > > + > > + � � � /* Set 902x in hardware TPI mode on and jump out of D3 state */ > > + � � � if (sii902x_write(client, 0xc7, 0x00) < 0) { > > + � � � � � � � dev_err(&client->dev, "SII902x: cound not find device\n"); > > + � � � � � � � return -ENODEV; > > Leaks priv. Same on other error paths. Jup, thanks for noting. There are some other bugs in the probe function, like not checking the return value of drm_encon_register(). Will fix > > > + � � � } > [...] > > > + > > + > > +static int sii902x_remove(struct i2c_client *client) > > +{ > > + � � � struct sii902x_priv *priv; > > + � � � int ret; > > + > > + � � � priv = i2c_get_clientdata(client); > > + > > + � � � ret = drm_encon_unregister(&priv->encon); > > + � � � if (ret) > > + � � � � � � � return ret; > > Leaks priv on error. and forgets to free_irq() Thanks Sascha -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 4/5] DRM: Add support for the sii902x HDMI/DVI encoder
On Tue, Jun 7, 2011 at 6:45 AM, Sascha Hauer wrote: > Signed-off-by: Sascha Hauer > --- > drivers/gpu/drm/Kconfig | 6 + > drivers/gpu/drm/i2c/Makefile | 3 + > drivers/gpu/drm/i2c/sii902x.c | 334 > + > 3 files changed, 343 insertions(+), 0 deletions(-) > create mode 100644 drivers/gpu/drm/i2c/sii902x.c > > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig > index bcd9a27..01d5444 100644 > --- a/drivers/gpu/drm/Kconfig > +++ b/drivers/gpu/drm/Kconfig > @@ -166,6 +166,12 @@ config DRM_SAVAGE > Choose this option if you have a Savage3D/4/SuperSavage/Pro/Twister > chipset. If M is selected the module will be called savage. > > +config DRM_I2C_SII902X > + tristate "sii902x" > + depends on DRM && I2C > + help > + Support for sii902x DVI/HDMI encoder chips > + > config DRM_IMX_IPUV3 > tristate "i.MX IPUv3" > depends on DRM && ARCH_MXC > diff --git a/drivers/gpu/drm/i2c/Makefile b/drivers/gpu/drm/i2c/Makefile > index 9286256..a7a8d40 100644 > --- a/drivers/gpu/drm/i2c/Makefile > +++ b/drivers/gpu/drm/i2c/Makefile > @@ -5,3 +5,6 @@ obj-$(CONFIG_DRM_I2C_CH7006) += ch7006.o > > sil164-y := sil164_drv.o > obj-$(CONFIG_DRM_I2C_SIL164) += sil164.o > + > +sii902x := sii902x_drv.o > +obj-$(CONFIG_DRM_I2C_SII902X) += sii902x.o > diff --git a/drivers/gpu/drm/i2c/sii902x.c b/drivers/gpu/drm/i2c/sii902x.c > new file mode 100644 > index 000..7928533 > --- /dev/null > +++ b/drivers/gpu/drm/i2c/sii902x.c > @@ -0,0 +1,334 @@ > +/* > + * Copyright (C) 2010 Francisco Jerez. Update the copyright? Alex > + * All Rights Reserved. > + * > + * Permission is hereby granted, free of charge, to any person obtaining > + * a copy of this software and associated documentation files (the > + * "Software"), to deal in the Software without restriction, including > + * without limitation the rights to use, copy, modify, merge, publish, > + * distribute, sublicense, and/or sell copies of the Software, and to > + * permit persons to whom the Software is furnished to do so, subject to > + * the following conditions: > + * > + * The above copyright notice and this permission notice (including the > + * next paragraph) shall be included in all copies or substantial > + * portions of the Software. > + * > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, > + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF > + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. > + * IN NO EVENT SHALL THE COPYRIGHT OWNER(S) AND/OR ITS SUPPLIERS BE > + * LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION > + * OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION > + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. > + * > + */ > + > +#include > +#include > +#include > +#include > + > +struct sii902x_encoder_params { > +}; > + > +struct sii902x_priv { > + struct sii902x_encoder_params config; > + struct i2c_client *client; > + struct drm_encoder_connector encon; > +}; > + > +#define to_sii902x(x) container_of(x, struct sii902x_priv, encon) > + > +static int sii902x_write(struct i2c_client *client, uint8_t addr, uint8_t > val) > +{ > + int ret; > + > + ret = i2c_smbus_write_byte_data(client, addr, val); > + if (ret) { > + dev_dbg(&client->dev, "%s failed with %d\n", __func__, ret); > + } > + return ret; > +} > + > +static uint8_t sii902x_read(struct i2c_client *client, uint8_t addr) > +{ > + int dat; > + > + dat = i2c_smbus_read_byte_data(client, addr); > + > + return dat; > +} > + > +static int hdmi_cap = 0; /* FIXME */ > + > +static void sii902x_poweron(struct sii902x_priv *priv) > +{ > + struct i2c_client *client = priv->client; > + > + /* Turn on DVI or HDMI */ > + if (hdmi_cap) > + sii902x_write(client, 0x1A, 0x01 | 4); > + else > + sii902x_write(client, 0x1A, 0x00); > + > + return; > +} > + > +static void sii902x_poweroff(struct sii902x_priv *priv) > +{ > + struct i2c_client *client = priv->client; > + > + /* disable tmds before changing resolution */ > + if (hdmi_cap) > + sii902x_write(client, 0x1A, 0x11); > + else > + sii902x_write(client, 0x1A, 0x10); > + > + return; > +} > + > +static int sii902x_get_modes(struct drm_encoder_connector *encon) > +{ > + struct sii902x_priv *priv = to_sii902x(encon); > + struct i2c_client *client = priv->client; > + struct i2c_adapter *adap = client->adapter; > + struct drm_connector *connector = &encon->connector; > + struct edid *edid; > + int ret; > + int old, dat, cnt = 100; > + > + old = sii902x_read(client, 0x1A); > + > + sii902x_write(client, 0x1A, old | 0x4); > + do { > + cnt--; > + msleep(
Re: [PATCH 4/5] DRM: Add support for the sii902x HDMI/DVI encoder
2011/6/7 Sascha Hauer : [...] > --- /dev/null > +++ b/drivers/gpu/drm/i2c/sii902x.c > @@ -0,0 +1,334 @@ [...] > +static int sii902x_write(struct i2c_client *client, uint8_t addr, uint8_t > val) > +{ > + int ret; > + > + ret = i2c_smbus_write_byte_data(client, addr, val); > + if (ret) { > + dev_dbg(&client->dev, "%s failed with %d\n", __func__, ret); > + } > + return ret; > +} Return value is never tested. > +static irqreturn_t sii902x_detect_handler(int irq, void *data) > +{ > + struct sii902x_priv *priv = data; > + struct i2c_client *client = priv->client; > + int dat; > + > + dat = sii902x_read(client, 0x3D); > + if (dat & 0x1) { > + /* cable connection changes */ > + if (dat & 0x4) { > + printk("plugin\n"); > + } else { > + printk("plugout\n"); > + } Missing code? > + } > + sii902x_write(client, 0x3D, dat); > + > + return IRQ_HANDLED; > +} [...] > +/* I2C driver functions */ > + > +static int > +sii902x_probe(struct i2c_client *client, const struct i2c_device_id *id) > +{ > + int dat, ret; > + struct sii902x_priv *priv; > + const char *drm_name = "imx-drm.0"; /* FIXME: pass from pdata */ > + int encon_id = 0; /* FIXME: pass from pdata */ > + > + priv = kzalloc(sizeof(*priv), GFP_KERNEL); > + if (!priv) > + return -ENOMEM; > + > + priv->client = client; > + > + /* Set 902x in hardware TPI mode on and jump out of D3 state */ > + if (sii902x_write(client, 0xc7, 0x00) < 0) { > + dev_err(&client->dev, "SII902x: cound not find device\n"); > + return -ENODEV; Leaks priv. Same on other error paths. > + } [...] > + > + > +static int sii902x_remove(struct i2c_client *client) > +{ > + struct sii902x_priv *priv; > + int ret; > + > + priv = i2c_get_clientdata(client); > + > + ret = drm_encon_unregister(&priv->encon); > + if (ret) > + return ret; Leaks priv on error. > + > + kfree(priv); > + > + return 0; > +} [...] Best Regards, Michał Mirosław ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 4/5] DRM: Add support for the sii902x HDMI/DVI encoder
On Fri, Jul 08, 2011 at 11:01:18PM +0200, Michał Mirosław wrote: > 2011/6/7 Sascha Hauer : > [...] > > --- /dev/null > > +++ b/drivers/gpu/drm/i2c/sii902x.c > > @@ -0,0 +1,334 @@ > [...] > > +static int sii902x_write(struct i2c_client *client, uint8_t addr, uint8_t > > val) > > +{ > > + � � � int ret; > > + > > + � � � ret = i2c_smbus_write_byte_data(client, addr, val); > > + � � � if (ret) { > > + � � � � � � � dev_dbg(&client->dev, "%s failed with %d\n", __func__, ret); > > + � � � } > > + � � � return ret; > > +} > > Return value is never tested. Yes, but there is not much I can do about it. This function is called from void functions most of the time, so I can't even forward the error. > > > +static irqreturn_t sii902x_detect_handler(int irq, void *data) > > +{ > > + � � � struct sii902x_priv *priv = data; > > + � � � struct i2c_client *client = priv->client; > > + � � � int dat; > > + > > + � � � dat = sii902x_read(client, 0x3D); > > + � � � if (dat & 0x1) { > > + � � � � � � � /* cable connection changes */ > > + � � � � � � � if (dat & 0x4) { > > + � � � � � � � � � � � printk("plugin\n"); > > + � � � � � � � } else { > > + � � � � � � � � � � � printk("plugout\n"); > > + � � � � � � � } > > Missing code? Yes. I will either remove interrupt support or put the missing code in. > > > + � � � } > > + � � � sii902x_write(client, 0x3D, dat); > > + > > + � � � return IRQ_HANDLED; > > +} > [...] > > +/* I2C driver functions */ > > + > > +static int > > +sii902x_probe(struct i2c_client *client, const struct i2c_device_id *id) > > +{ > > + � � � int dat, ret; > > + � � � struct sii902x_priv *priv; > > + � � � const char *drm_name = "imx-drm.0"; /* FIXME: pass from pdata */ > > + � � � int encon_id = 0; /* FIXME: pass from pdata */ > > + > > + � � � priv = kzalloc(sizeof(*priv), GFP_KERNEL); > > + � � � if (!priv) > > + � � � � � � � return -ENOMEM; > > + > > + � � � priv->client = client; > > + > > + � � � /* Set 902x in hardware TPI mode on and jump out of D3 state */ > > + � � � if (sii902x_write(client, 0xc7, 0x00) < 0) { > > + � � � � � � � dev_err(&client->dev, "SII902x: cound not find device\n"); > > + � � � � � � � return -ENODEV; > > Leaks priv. Same on other error paths. Jup, thanks for noting. There are some other bugs in the probe function, like not checking the return value of drm_encon_register(). Will fix > > > + � � � } > [...] > > > + > > + > > +static int sii902x_remove(struct i2c_client *client) > > +{ > > + � � � struct sii902x_priv *priv; > > + � � � int ret; > > + > > + � � � priv = i2c_get_clientdata(client); > > + > > + � � � ret = drm_encon_unregister(&priv->encon); > > + � � � if (ret) > > + � � � � � � � return ret; > > Leaks priv on error. and forgets to free_irq() Thanks Sascha -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel