Re: [PATCH 2/4] drm/i2c: nxp-tda998x (v2)
On Thu, Jan 24, 2013 at 5:57 AM, Daniel Vetter wrote:
> On Tue, Jan 22, 2013 at 04:36:23PM -0600, Rob Clark wrote:
>> Driver for the NXP TDA998X i2c hdmi encoder slave.
>>
>> v1: original
>> v2: fix npix/nline programming
>>
>> Signed-off-by: Rob Clark
>
> Just one bikeshed, otherwise
>
> Reviewed-by: Daniel Vetter
>
> [cut]
>
>> +static void
>> +reg_set(struct drm_encoder *encoder, uint16_t reg, uint8_t val)
>> +{
>> + reg_write(encoder, reg, reg_read(encoder, reg) | val);
>> +}
>> +
>> +static void
>> +reg_clear(struct drm_encoder *encoder, uint16_t reg, uint8_t val)
>> +{
>> + reg_write(encoder, reg, reg_read(encoder, reg) & ~val);
>> +}
>
> What about drivers/base/regmap? I haven't looked to closely yet and never
> used it in code, but there's a presentation [1] and it sounds like it
> provides some nice (and more important standardized) helper stuff for
> debug, tracing, ...
>
> Since encoder slave drivers tend to be utterly boring register bashing and
> we expect tons of time, I think high levels of standardization would be
> really useful. Care to look into this a bit?
I did look at regmap.. what convinced me against using it was that if
you don't use cached mode, it ends up writing the page selector
register for every read/write. And I don't have enough actual
documentation about this nxp part to know the reset values of all the
registers in order to use caching.
BR,
-R
> Cheers, Daniel
>
> 1: http://free-electrons.com/blog/fosdem2012-videos/
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/4] drm/i2c: nxp-tda998x (v2)
On Tue, Jan 22, 2013 at 04:36:23PM -0600, Rob Clark wrote:
> Driver for the NXP TDA998X i2c hdmi encoder slave.
>
> v1: original
> v2: fix npix/nline programming
>
> Signed-off-by: Rob Clark
Just one bikeshed, otherwise
Reviewed-by: Daniel Vetter
[cut]
> +static void
> +reg_set(struct drm_encoder *encoder, uint16_t reg, uint8_t val)
> +{
> + reg_write(encoder, reg, reg_read(encoder, reg) | val);
> +}
> +
> +static void
> +reg_clear(struct drm_encoder *encoder, uint16_t reg, uint8_t val)
> +{
> + reg_write(encoder, reg, reg_read(encoder, reg) & ~val);
> +}
What about drivers/base/regmap? I haven't looked to closely yet and never
used it in code, but there's a presentation [1] and it sounds like it
provides some nice (and more important standardized) helper stuff for
debug, tracing, ...
Since encoder slave drivers tend to be utterly boring register bashing and
we expect tons of time, I think high levels of standardization would be
really useful. Care to look into this a bit?
Cheers, Daniel
1: http://free-electrons.com/blog/fosdem2012-videos/
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/4] drm/i2c: nxp-tda998x (v2)
On Wed, Jan 23, 2013 at 3:42 AM, Jean-Francois Moine wrote:
> On Tue, 22 Jan 2013 16:36:23 -0600
> Rob Clark wrote:
>
>> Driver for the NXP TDA998X i2c hdmi encoder slave.
>>
>> v1: original
>> v2: fix npix/nline programming
>>
>> Signed-off-by: Rob Clark
>> ---
>> drivers/gpu/drm/i2c/Makefile | 3 +
>> drivers/gpu/drm/i2c/tda998x_drv.c | 908
>> ++
>> 2 files changed, 911 insertions(+)
>> create mode 100644 drivers/gpu/drm/i2c/tda998x_drv.c
> [snip]
>> diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c
>> b/drivers/gpu/drm/i2c/tda998x_drv.c
>> new file mode 100644
>> index 000..02054e8
>> --- /dev/null
>> +++ b/drivers/gpu/drm/i2c/tda998x_drv.c
>> @@ -0,0 +1,908 @@
> [snip]
>> +static void __exit
>> +tda998x_exit(void)
>> +{
>> + DBG("");
>> + drm_i2c_encoder_unregister(&tda998x_driver);
>> +}
>> +
>> +MODULE_DESCRIPTION("NXP Semiconductors TDA998X TMDS transmitter driver");
>> +
>> +MODULE_AUTHOR("Rob Clark > +MODULE_DESCRIPTION("NXP Semiconductors TDA998X HDMI Encoder");
>> +MODULE_LICENSE("GPL");
>> +
>> +module_init(tda998x_init);
>> +module_exit(tda998x_exit);
>
> There are 2 MODULE_DESCRIPTION().
oh, whoops.. I'll fix that
BR,
-R
> --
> Ken ar c'hentañ | ** Breizh ha Linux atav! **
> Jef | http://moinejf.free.fr/
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/4] drm/i2c: nxp-tda998x (v2)
On Tue, 22 Jan 2013 16:36:23 -0600
Rob Clark wrote:
> Driver for the NXP TDA998X i2c hdmi encoder slave.
>
> v1: original
> v2: fix npix/nline programming
>
> Signed-off-by: Rob Clark
> ---
> drivers/gpu/drm/i2c/Makefile | 3 +
> drivers/gpu/drm/i2c/tda998x_drv.c | 908
> ++
> 2 files changed, 911 insertions(+)
> create mode 100644 drivers/gpu/drm/i2c/tda998x_drv.c
[snip]
> diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c
> b/drivers/gpu/drm/i2c/tda998x_drv.c
> new file mode 100644
> index 000..02054e8
> --- /dev/null
> +++ b/drivers/gpu/drm/i2c/tda998x_drv.c
> @@ -0,0 +1,908 @@
[snip]
> +static void __exit
> +tda998x_exit(void)
> +{
> + DBG("");
> + drm_i2c_encoder_unregister(&tda998x_driver);
> +}
> +
> +MODULE_DESCRIPTION("NXP Semiconductors TDA998X TMDS transmitter driver");
> +
> +MODULE_AUTHOR("Rob Clark +MODULE_DESCRIPTION("NXP Semiconductors TDA998X HDMI Encoder");
> +MODULE_LICENSE("GPL");
> +
> +module_init(tda998x_init);
> +module_exit(tda998x_exit);
There are 2 MODULE_DESCRIPTION().
--
Ken ar c'hentañ | ** Breizh ha Linux atav! **
Jef | http://moinejf.free.fr/
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/4] drm/i2c: nxp-tda998x (v2)
Op 22 jan. 2013, om 23:36 heeft Rob Clark het volgende geschreven: > Driver for the NXP TDA998X i2c hdmi encoder slave. > > v1: original > v2: fix npix/nline programming > > Signed-off-by: Rob Clark Tested on a beaglebone-black: Tested-by: Koen Kooi -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
