Re: [PATCH 2/2] [RFC v2 with seqcount] reservation: add suppport for read-only access using rcu
op 11-04-14 21:30, Thomas Hellstrom schreef: Hi! On 04/11/2014 08:09 PM, Maarten Lankhorst wrote: op 11-04-14 12:11, Thomas Hellstrom schreef: On 04/11/2014 11:24 AM, Maarten Lankhorst wrote: op 11-04-14 10:38, Thomas Hellstrom schreef: Hi, Maarten. Here I believe we encounter a lot of locking inconsistencies. First, it seems you're use a number of pointers as RCU pointers without annotating them as such and use the correct rcu macros when assigning those pointers. Some pointers (like the pointers in the shared fence list) are both used as RCU pointers (in dma_buf_poll()) for example, or considered protected by the seqlock (reservation_object_get_fences_rcu()), which I believe is OK, but then the pointers must be assigned using the correct rcu macros. In the memcpy in reservation_object_get_fences_rcu() we might get away with an ugly typecast, but with a verbose comment that the pointers are considered protected by the seqlock at that location. So I've updated (attached) the headers with proper __rcu annotation and locking comments according to how they are being used in the various reading functions. I believe if we want to get rid of this we need to validate those pointers using the seqlock as well. This will generate a lot of sparse warnings in those places needing rcu_dereference() rcu_assign_pointer() rcu_dereference_protected() With this I think we can get rid of all ACCESS_ONCE macros: It's not needed when the rcu_x() macros are used, and it's never needed for the members protected by the seqlock, (provided that the seq is tested). The only place where I think that's *not* the case is at the krealloc in reservation_object_get_fences_rcu(). Also I have some more comments in the reservation_object_get_fences_rcu() function below: I felt that the barriers needed for rcu were already provided by checking the seqcount lock. But looking at rcu_dereference makes it seem harmless to add it in more places, it handles the ACCESS_ONCE and barrier() for us. And it makes the code more maintainable, and helps sparse doing a lot of checking for us. I guess we can tolerate a couple of extra barriers for that. We could probably get away with using RCU_INIT_POINTER on the writer side, because the smp_wmb is already done by arranging seqcount updates correctly. Hmm. yes, probably. At least in the replace function. I think if we do it in other places, we should add comments as to where the smp_wmb() is located, for future reference. Also I saw in a couple of places where you're checking the shared pointers, you're not checking for NULL pointers, which I guess may happen if shared_count and pointers are not in full sync? No, because shared_count is protected with seqcount. I only allow appending to the array, so when shared_count is validated by seqcount it means that the [0...shared_count) indexes are valid and non-null. What could happen though is that the fence at a specific index is updated with another one from the same context, but that's harmless. Hmm. Shouldn't we have a way to clean signaled fences from reservation objects? Perhaps when we attach a new fence, or after a wait with ww_mutex held? Otherwise we'd have a lot of completely unused fence objects hanging around for no reason. I don't think we need to be as picky as TTM, but I think we should do something? Calling reservation_object_add_excl_fence with a NULL fence works, I do this in ttm_bo_wait(). It requires ww_mutex. ~Maarten -- To unsubscribe from this list: send the line unsubscribe linux-media 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/2] [RFC v2 with seqcount] reservation: add suppport for read-only access using rcu
op 11-04-14 21:35, Thomas Hellstrom schreef: On 04/11/2014 08:09 PM, Maarten Lankhorst wrote: op 11-04-14 12:11, Thomas Hellstrom schreef: On 04/11/2014 11:24 AM, Maarten Lankhorst wrote: op 11-04-14 10:38, Thomas Hellstrom schreef: Hi, Maarten. Here I believe we encounter a lot of locking inconsistencies. First, it seems you're use a number of pointers as RCU pointers without annotating them as such and use the correct rcu macros when assigning those pointers. Some pointers (like the pointers in the shared fence list) are both used as RCU pointers (in dma_buf_poll()) for example, or considered protected by the seqlock (reservation_object_get_fences_rcu()), which I believe is OK, but then the pointers must be assigned using the correct rcu macros. In the memcpy in reservation_object_get_fences_rcu() we might get away with an ugly typecast, but with a verbose comment that the pointers are considered protected by the seqlock at that location. So I've updated (attached) the headers with proper __rcu annotation and locking comments according to how they are being used in the various reading functions. I believe if we want to get rid of this we need to validate those pointers using the seqlock as well. This will generate a lot of sparse warnings in those places needing rcu_dereference() rcu_assign_pointer() rcu_dereference_protected() With this I think we can get rid of all ACCESS_ONCE macros: It's not needed when the rcu_x() macros are used, and it's never needed for the members protected by the seqlock, (provided that the seq is tested). The only place where I think that's *not* the case is at the krealloc in reservation_object_get_fences_rcu(). Also I have some more comments in the reservation_object_get_fences_rcu() function below: I felt that the barriers needed for rcu were already provided by checking the seqcount lock. But looking at rcu_dereference makes it seem harmless to add it in more places, it handles the ACCESS_ONCE and barrier() for us. And it makes the code more maintainable, and helps sparse doing a lot of checking for us. I guess we can tolerate a couple of extra barriers for that. We could probably get away with using RCU_INIT_POINTER on the writer side, because the smp_wmb is already done by arranging seqcount updates correctly. Hmm. yes, probably. At least in the replace function. I think if we do it in other places, we should add comments as to where the smp_wmb() is located, for future reference. Also I saw in a couple of places where you're checking the shared pointers, you're not checking for NULL pointers, which I guess may happen if shared_count and pointers are not in full sync? No, because shared_count is protected with seqcount. I only allow appending to the array, so when shared_count is validated by seqcount it means that the [0...shared_count) indexes are valid and non-null. What could happen though is that the fence at a specific index is updated with another one from the same context, but that's harmless. Hmm, doesn't attaching an exclusive fence clear all shared fence pointers from under a reader? No, for that reason. It only resets shared_count to 0. This is harmless because the shared fence pointers are still valid long enough because of RCU delayed deletion. fence_get_rcu will fail when the refcount has dropped to zero. This is enough of a check to prevent errors, so there's no need to explicitly clear the fence pointers. ~Maarten -- To unsubscribe from this list: send the line unsubscribe linux-media 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/2] [RFC v2 with seqcount] reservation: add suppport for read-only access using rcu
On 04/14/2014 09:42 AM, Maarten Lankhorst wrote: op 11-04-14 21:35, Thomas Hellstrom schreef: On 04/11/2014 08:09 PM, Maarten Lankhorst wrote: op 11-04-14 12:11, Thomas Hellstrom schreef: On 04/11/2014 11:24 AM, Maarten Lankhorst wrote: op 11-04-14 10:38, Thomas Hellstrom schreef: Hi, Maarten. Here I believe we encounter a lot of locking inconsistencies. First, it seems you're use a number of pointers as RCU pointers without annotating them as such and use the correct rcu macros when assigning those pointers. Some pointers (like the pointers in the shared fence list) are both used as RCU pointers (in dma_buf_poll()) for example, or considered protected by the seqlock (reservation_object_get_fences_rcu()), which I believe is OK, but then the pointers must be assigned using the correct rcu macros. In the memcpy in reservation_object_get_fences_rcu() we might get away with an ugly typecast, but with a verbose comment that the pointers are considered protected by the seqlock at that location. So I've updated (attached) the headers with proper __rcu annotation and locking comments according to how they are being used in the various reading functions. I believe if we want to get rid of this we need to validate those pointers using the seqlock as well. This will generate a lot of sparse warnings in those places needing rcu_dereference() rcu_assign_pointer() rcu_dereference_protected() With this I think we can get rid of all ACCESS_ONCE macros: It's not needed when the rcu_x() macros are used, and it's never needed for the members protected by the seqlock, (provided that the seq is tested). The only place where I think that's *not* the case is at the krealloc in reservation_object_get_fences_rcu(). Also I have some more comments in the reservation_object_get_fences_rcu() function below: I felt that the barriers needed for rcu were already provided by checking the seqcount lock. But looking at rcu_dereference makes it seem harmless to add it in more places, it handles the ACCESS_ONCE and barrier() for us. And it makes the code more maintainable, and helps sparse doing a lot of checking for us. I guess we can tolerate a couple of extra barriers for that. We could probably get away with using RCU_INIT_POINTER on the writer side, because the smp_wmb is already done by arranging seqcount updates correctly. Hmm. yes, probably. At least in the replace function. I think if we do it in other places, we should add comments as to where the smp_wmb() is located, for future reference. Also I saw in a couple of places where you're checking the shared pointers, you're not checking for NULL pointers, which I guess may happen if shared_count and pointers are not in full sync? No, because shared_count is protected with seqcount. I only allow appending to the array, so when shared_count is validated by seqcount it means that the [0...shared_count) indexes are valid and non-null. What could happen though is that the fence at a specific index is updated with another one from the same context, but that's harmless. Hmm, doesn't attaching an exclusive fence clear all shared fence pointers from under a reader? No, for that reason. It only resets shared_count to 0. Ah. OK. I guess I didn't read the code carefully enough. Thanks, Thomas ~Maarten -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 03/21] smiapp: Fix determining the need for 8-bit read access
8-bit reads are needed in some cases; however the condition used was wrong. Regular access (register width) was used if: len == SMIAPP_REG_8BIT !only8 This causes 8-bit read access to be used always. The operator should be || instead: regular access can be used for 8-bit reads OR if allowed otherwise. Signed-off-by: Sakari Ailus sakari.ai...@linux.intel.com --- drivers/media/i2c/smiapp/smiapp-regs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/i2c/smiapp/smiapp-regs.c b/drivers/media/i2c/smiapp/smiapp-regs.c index 5d0151a..c2db205 100644 --- a/drivers/media/i2c/smiapp/smiapp-regs.c +++ b/drivers/media/i2c/smiapp/smiapp-regs.c @@ -172,7 +172,7 @@ static int __smiapp_read(struct smiapp_sensor *sensor, u32 reg, u32 *val, len != SMIAPP_REG_32BIT) return -EINVAL; - if (len == SMIAPP_REG_8BIT !only8) + if (len == SMIAPP_REG_8BIT || !only8) rval = smiapp_read(sensor, (u16)reg, len, val); else rval = smiapp_read_8only(sensor, (u16)reg, len, val); -- 1.8.3.2 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 09/21] smiapp: Use %u for printing u32 value
Signed-off-by: Sakari Ailus sakari.ai...@linux.intel.com --- drivers/media/i2c/smiapp/smiapp-core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/i2c/smiapp/smiapp-core.c b/drivers/media/i2c/smiapp/smiapp-core.c index 02041cc..3af8df8 100644 --- a/drivers/media/i2c/smiapp/smiapp-core.c +++ b/drivers/media/i2c/smiapp/smiapp-core.c @@ -606,7 +606,7 @@ static int smiapp_get_limits(struct smiapp_sensor *sensor, int const *limit, if (rval) return rval; sensor-limits[limit[i]] = val; - dev_dbg(client-dev, 0x%8.8x \%s\ = %d, 0x%x\n, + dev_dbg(client-dev, 0x%8.8x \%s\ = %u, 0x%x\n, smiapp_reg_limits[limit[i]].addr, smiapp_reg_limits[limit[i]].what, val, val); } -- 1.8.3.2 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 04/21] smiapp: Add a macro for constructing 8-bit quirk registers
Signed-off-by: Sakari Ailus sakari.ai...@linux.intel.com --- drivers/media/i2c/smiapp/smiapp-quirk.h | 6 ++ 1 file changed, 6 insertions(+) diff --git a/drivers/media/i2c/smiapp/smiapp-quirk.h b/drivers/media/i2c/smiapp/smiapp-quirk.h index 4f65c4e..96a253e 100644 --- a/drivers/media/i2c/smiapp/smiapp-quirk.h +++ b/drivers/media/i2c/smiapp/smiapp-quirk.h @@ -56,6 +56,12 @@ struct smiapp_reg_8 { void smiapp_replace_limit(struct smiapp_sensor *sensor, u32 limit, u32 val); +#define SMIAPP_MK_QUIRK_REG_8(_reg, _val) \ + { \ + .reg = (u16)_reg, \ + .val = _val,\ + } + #define smiapp_call_quirk(_sensor, _quirk, ...) \ (_sensor-minfo.quirk \ _sensor-minfo.quirk-_quirk ? \ -- 1.8.3.2 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 08/21] smiapp: Make PLL (quirk) flags a function
This is more flexible. Quirk flags may be affected by configuration. Signed-off-by: Sakari Ailus sakari.ai...@linux.intel.com --- drivers/media/i2c/smiapp/smiapp-core.c | 4 ++-- drivers/media/i2c/smiapp/smiapp-quirk.c | 7 ++- drivers/media/i2c/smiapp/smiapp-quirk.h | 2 +- 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/drivers/media/i2c/smiapp/smiapp-core.c b/drivers/media/i2c/smiapp/smiapp-core.c index 23f2c4d..02041cc 100644 --- a/drivers/media/i2c/smiapp/smiapp-core.c +++ b/drivers/media/i2c/smiapp/smiapp-core.c @@ -2617,8 +2617,8 @@ static int smiapp_registered(struct v4l2_subdev *subdev) pll-bus_type = SMIAPP_PLL_BUS_TYPE_CSI2; pll-csi2.lanes = sensor-platform_data-lanes; pll-ext_clk_freq_hz = sensor-platform_data-ext_clk; - if (sensor-minfo.quirk) - pll-flags = sensor-minfo.quirk-pll_flags; + pll-flags = smiapp_call_quirk(sensor, pll_flags); + /* Profile 0 sensors have no separate OP clock branch. */ if (sensor-minfo.smiapp_profile == SMIAPP_PROFILE_0) pll-flags |= SMIAPP_PLL_FLAG_NO_OP_CLOCKS; diff --git a/drivers/media/i2c/smiapp/smiapp-quirk.c b/drivers/media/i2c/smiapp/smiapp-quirk.c index bd2f8a7..e0bee87 100644 --- a/drivers/media/i2c/smiapp/smiapp-quirk.c +++ b/drivers/media/i2c/smiapp/smiapp-quirk.c @@ -220,12 +220,17 @@ static int jt8ev1_post_streamoff(struct smiapp_sensor *sensor) return smiapp_write_8(sensor, 0x3328, 0x80); } +static unsigned long jt8ev1_pll_flags(struct smiapp_sensor *sensor) +{ + return SMIAPP_PLL_FLAG_OP_PIX_CLOCK_PER_LANE; +} + const struct smiapp_quirk smiapp_jt8ev1_quirk = { .limits = jt8ev1_limits, .post_poweron = jt8ev1_post_poweron, .pre_streamon = jt8ev1_pre_streamon, .post_streamoff = jt8ev1_post_streamoff, - .pll_flags = SMIAPP_PLL_FLAG_OP_PIX_CLOCK_PER_LANE, + .pll_flags = jt8ev1_pll_flags, }; static int tcm8500md_limits(struct smiapp_sensor *sensor) diff --git a/drivers/media/i2c/smiapp/smiapp-quirk.h b/drivers/media/i2c/smiapp/smiapp-quirk.h index ea8231c6..dddb62b 100644 --- a/drivers/media/i2c/smiapp/smiapp-quirk.h +++ b/drivers/media/i2c/smiapp/smiapp-quirk.h @@ -41,8 +41,8 @@ struct smiapp_quirk { int (*post_poweron)(struct smiapp_sensor *sensor); int (*pre_streamon)(struct smiapp_sensor *sensor); int (*post_streamoff)(struct smiapp_sensor *sensor); + unsigned long (*pll_flags)(struct smiapp_sensor *sensor); unsigned long flags; - unsigned long pll_flags; }; #define SMIAPP_QUIRK_FLAG_8BIT_READ_ONLY (1 0) -- 1.8.3.2 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 00/12] smiapp and smiapp-pll quirk improvements, fixes
Hi folks, This is the second version of the smiapp and smiapp-pll quirk improvement patchset. The first version can be found here: URL:http://www.spinics.net/lists/linux-media/msg75538.html Changes since v1: - Fix a compiler warning in smiapp-pll.c added by the patch changing limits to 64 bits. - Add a patch that contains macros to access register address, width and flags (smiapp: Define macros for obtaining properties of register definitions). - Add a patch to remove the old register quirks (smiapp: Remove unused quirk register functionality). - Make register diversion quirk more generic. Access can be now avoided altogether; up to the quirk implementation. Quirk functions can perform further register accesses. Additional functions are provided for quirk functions to avoid checking for further quirks (no quirks for quirks). - Add a patch to rename smia prefix still used somehwere as smiapp for consistency. This patchset contains PLL quirk improvements to take quirks in some implementations into account, as well as make the quirk mechanisms more flexible. The driver core is mostly unaffected by these changes. The PLL tree calculation itself is concerned less with the factual frequencies but focuses on producing multipliers and dividers that are valid for the hardware. Quirk flags are primarily used to convert input and output parameters. The limit values are also made 64 bits; 64-bit values are needed in more generic case when floating point numbers are converted to fixed point. There are some miscellaneous fixes as well. -- Kind regards, Sakari -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 16/21] smiapp-pll: Add quirk for op clk divisor == bits per pixel / 2
For some sensors in some configurations the effective value of op clk div is bits per pixel divided by two. The output clock is correctly calculated whereas some of the rest of the clock tree uses higher clocks than calculated. This also limits the bpp to even values if the number of lanes is four. Signed-off-by: Sakari Ailus sakari.ai...@linux.intel.com --- drivers/media/i2c/smiapp-pll.c | 10 ++ drivers/media/i2c/smiapp-pll.h | 2 ++ 2 files changed, 12 insertions(+) diff --git a/drivers/media/i2c/smiapp-pll.c b/drivers/media/i2c/smiapp-pll.c index 6bde587..aca0ed7 100644 --- a/drivers/media/i2c/smiapp-pll.c +++ b/drivers/media/i2c/smiapp-pll.c @@ -208,6 +208,8 @@ static int __smiapp_pll_calculate(struct device *dev, div_u64(pll-pll_op_clk_freq_hz, pll-op_sys_clk_div); pll-op_pix_clk_div = pll-bits_per_pixel; + if (pll-flags SMIAPP_PLL_FLAG_OP_PIX_DIV_HALF) + pll-op_pix_clk_div /= 2; dev_dbg(dev, op_pix_clk_div: %u\n, pll-op_pix_clk_div); pll-op_pix_clk_freq_hz = @@ -417,6 +419,14 @@ int smiapp_pll_calculate(struct device *dev, return -EINVAL; } + /* +* Half op pix divisor will give us double the rate compared +* to the regular case. Thus divide the desired pll op clock +* frequency by two. +*/ + if (pll-flags SMIAPP_PLL_FLAG_OP_PIX_DIV_HALF) + pll-pll_op_clk_freq_hz /= 2; + /* Figure out limits for pre-pll divider based on extclk */ dev_dbg(dev, min / max pre_pll_clk_div: %u / %u\n, limits-min_pre_pll_clk_div, limits-max_pre_pll_clk_div); diff --git a/drivers/media/i2c/smiapp-pll.h b/drivers/media/i2c/smiapp-pll.h index a25f550..02d11db 100644 --- a/drivers/media/i2c/smiapp-pll.h +++ b/drivers/media/i2c/smiapp-pll.h @@ -36,6 +36,8 @@ #define SMIAPP_PLL_FLAG_NO_OP_CLOCKS (1 1) /* the pre-pll div may be odd */ #define SMIAPP_PLL_FLAG_ALLOW_ODD_PRE_PLL_CLK_DIV (1 2) +/* op pix div value is half of the bits-per-pixel value */ +#define SMIAPP_PLL_FLAG_OP_PIX_DIV_HALF(1 3) struct smiapp_pll { /* input values */ -- 1.8.3.2 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 13/21] smiapp-pll: Use 64-bit types limits
Limits may exceed the value range of 32-bit unsigned integers. Thus use 64 bits for all of them. Signed-off-by: Sakari Ailus sakari.ai...@linux.intel.com --- drivers/media/i2c/smiapp-pll.c | 72 +++--- drivers/media/i2c/smiapp-pll.h | 20 ++-- 2 files changed, 50 insertions(+), 42 deletions(-) diff --git a/drivers/media/i2c/smiapp-pll.c b/drivers/media/i2c/smiapp-pll.c index d14af5c..ec9f8bb 100644 --- a/drivers/media/i2c/smiapp-pll.c +++ b/drivers/media/i2c/smiapp-pll.c @@ -28,6 +28,11 @@ #include smiapp-pll.h +static inline uint64_t div_u64_round_up(uint64_t dividend, uint32_t divisor) +{ + return div_u64(dividend + divisor - 1, divisor); +} + /* Return an even number or one. */ static inline uint32_t clk_div_even(uint32_t a) { @@ -52,13 +57,14 @@ static inline uint32_t is_one_or_even(uint32_t a) return 1; } -static int bounds_check(struct device *dev, uint32_t val, - uint32_t min, uint32_t max, char *str) +static int bounds_check(struct device *dev, uint64_t val, + uint64_t min, uint64_t max, char *str) { if (val = min val = max) return 0; - dev_dbg(dev, %s out of bounds: %d (%d--%d)\n, str, val, min, max); + dev_dbg(dev, %s out of bounds: %llu (%llu--%llu)\n, str, val, min, + max); return -EINVAL; } @@ -75,15 +81,15 @@ static void print_pll(struct device *dev, struct smiapp_pll *pll) dev_dbg(dev, vt_pix_clk_div \t%u\n, pll-vt_pix_clk_div); dev_dbg(dev, ext_clk_freq_hz \t%u\n, pll-ext_clk_freq_hz); - dev_dbg(dev, pll_ip_clk_freq_hz \t%u\n, pll-pll_ip_clk_freq_hz); - dev_dbg(dev, pll_op_clk_freq_hz \t%u\n, pll-pll_op_clk_freq_hz); + dev_dbg(dev, pll_ip_clk_freq_hz \t%llu\n, pll-pll_ip_clk_freq_hz); + dev_dbg(dev, pll_op_clk_freq_hz \t%llu\n, pll-pll_op_clk_freq_hz); if (!(pll-flags SMIAPP_PLL_FLAG_NO_OP_CLOCKS)) { - dev_dbg(dev, op_sys_clk_freq_hz \t%u\n, + dev_dbg(dev, op_sys_clk_freq_hz \t%llu\n, pll-op_sys_clk_freq_hz); dev_dbg(dev, op_pix_clk_freq_hz \t%u\n, pll-op_pix_clk_freq_hz); } - dev_dbg(dev, vt_sys_clk_freq_hz \t%u\n, pll-vt_sys_clk_freq_hz); + dev_dbg(dev, vt_sys_clk_freq_hz \t%llu\n, pll-vt_sys_clk_freq_hz); dev_dbg(dev, vt_pix_clk_freq_hz \t%u\n, pll-vt_pix_clk_freq_hz); } @@ -131,10 +137,11 @@ static int __smiapp_pll_calculate(struct device *dev, more_mul_max); /* Don't go above max pll op frequency. */ more_mul_max = - min_t(uint32_t, + min_t(uint64_t, more_mul_max, - limits-max_pll_op_freq_hz - / (pll-ext_clk_freq_hz / pll-pre_pll_clk_div * mul)); + div_u64(limits-max_pll_op_freq_hz, + (pll-ext_clk_freq_hz / + pll-pre_pll_clk_div * mul))); dev_dbg(dev, more_mul_max: max_pll_op_freq_hz check: %u\n, more_mul_max); /* Don't go above the division capability of op sys clock divider. */ @@ -150,9 +157,9 @@ static int __smiapp_pll_calculate(struct device *dev, more_mul_max); /* Ensure we won't go below min_pll_op_freq_hz. */ - more_mul_min = DIV_ROUND_UP(limits-min_pll_op_freq_hz, - pll-ext_clk_freq_hz / pll-pre_pll_clk_div - * mul); + more_mul_min = div_u64_round_up( + limits-min_pll_op_freq_hz, + pll-ext_clk_freq_hz / pll-pre_pll_clk_div * mul); dev_dbg(dev, more_mul_min: min_pll_op_freq_hz check: %u\n, more_mul_min); /* Ensure we won't go below min_pll_multiplier. */ @@ -194,13 +201,13 @@ static int __smiapp_pll_calculate(struct device *dev, /* Derive pll_op_clk_freq_hz. */ pll-op_sys_clk_freq_hz = - pll-pll_op_clk_freq_hz / pll-op_sys_clk_div; + div_u64(pll-pll_op_clk_freq_hz, pll-op_sys_clk_div); pll-op_pix_clk_div = pll-bits_per_pixel; dev_dbg(dev, op_pix_clk_div: %u\n, pll-op_pix_clk_div); pll-op_pix_clk_freq_hz = - pll-op_sys_clk_freq_hz / pll-op_pix_clk_div; + div_u64(pll-op_sys_clk_freq_hz, pll-op_pix_clk_div); /* * Some sensors perform analogue binning and some do this @@ -235,9 +242,9 @@ static int __smiapp_pll_calculate(struct device *dev, /* Find smallest and biggest allowed vt divisor. */ dev_dbg(dev, min_vt_div: %u\n, min_vt_div); - min_vt_div = max(min_vt_div, -DIV_ROUND_UP(pll-pll_op_clk_freq_hz, - limits-vt.max_pix_clk_freq_hz)); + min_vt_div = max_t(uint32_t, min_vt_div, +
[PATCH v2 01/21] smiapp: Remove unused quirk register functionality
The quirk registers mechanism which allows register to have a static read access value from the sensor specific quirks, is not used. Remove it. It is to be replaced by a more generic register diversion quirk soon. Signed-off-by: Sakari Ailus sakari.ai...@linux.intel.com --- drivers/media/i2c/smiapp/smiapp-quirk.c | 46 - drivers/media/i2c/smiapp/smiapp-quirk.h | 10 --- drivers/media/i2c/smiapp/smiapp-regs.c | 4 --- drivers/media/i2c/smiapp/smiapp-regs.h | 5 4 files changed, 65 deletions(-) diff --git a/drivers/media/i2c/smiapp/smiapp-quirk.c b/drivers/media/i2c/smiapp/smiapp-quirk.c index bb8c506..4955289 100644 --- a/drivers/media/i2c/smiapp/smiapp-quirk.c +++ b/drivers/media/i2c/smiapp/smiapp-quirk.c @@ -61,52 +61,6 @@ void smiapp_replace_limit(struct smiapp_sensor *sensor, sensor-limits[limit] = val; } -bool smiapp_quirk_reg(struct smiapp_sensor *sensor, - u32 reg, u32 *val) -{ - struct i2c_client *client = v4l2_get_subdevdata(sensor-src-sd); - const struct smia_reg *sreg; - - if (!sensor-minfo.quirk) - return false; - - sreg = sensor-minfo.quirk-regs; - - if (!sreg) - return false; - - while (sreg-type) { - u16 type = reg 16; - u16 reg16 = reg; - - if (sreg-type != type || sreg-reg != reg16) { - sreg++; - continue; - } - - switch ((u8)type) { - case SMIA_REG_8BIT: - dev_dbg(client-dev, quirk: 0x%8.8x: 0x%2.2x\n, - reg, sreg-val); - break; - case SMIA_REG_16BIT: - dev_dbg(client-dev, quirk: 0x%8.8x: 0x%4.4x\n, - reg, sreg-val); - break; - case SMIA_REG_32BIT: - dev_dbg(client-dev, quirk: 0x%8.8x: 0x%8.8x\n, - reg, sreg-val); - break; - } - - *val = sreg-val; - - return true; - } - - return false; -} - static int jt8ew9_limits(struct smiapp_sensor *sensor) { if (sensor-minfo.revision_number_major 0x03) diff --git a/drivers/media/i2c/smiapp/smiapp-quirk.h b/drivers/media/i2c/smiapp/smiapp-quirk.h index 504a6d8..4f65c4e 100644 --- a/drivers/media/i2c/smiapp/smiapp-quirk.h +++ b/drivers/media/i2c/smiapp/smiapp-quirk.h @@ -41,7 +41,6 @@ struct smiapp_quirk { int (*post_poweron)(struct smiapp_sensor *sensor); int (*pre_streamon)(struct smiapp_sensor *sensor); int (*post_streamoff)(struct smiapp_sensor *sensor); - const struct smia_reg *regs; unsigned long flags; }; @@ -56,15 +55,6 @@ struct smiapp_reg_8 { void smiapp_replace_limit(struct smiapp_sensor *sensor, u32 limit, u32 val); -bool smiapp_quirk_reg(struct smiapp_sensor *sensor, - u32 reg, u32 *val); - -#define SMIAPP_MK_QUIRK_REG(_reg, _val) \ - { \ - .type = (_reg 16), \ - .reg = (u16)_reg, \ - .val = _val,\ - } #define smiapp_call_quirk(_sensor, _quirk, ...) \ (_sensor-minfo.quirk \ diff --git a/drivers/media/i2c/smiapp/smiapp-regs.c b/drivers/media/i2c/smiapp/smiapp-regs.c index 4fac32c..e01644c 100644 --- a/drivers/media/i2c/smiapp/smiapp-regs.c +++ b/drivers/media/i2c/smiapp/smiapp-regs.c @@ -172,9 +172,6 @@ static int __smiapp_read(struct smiapp_sensor *sensor, u32 reg, u32 *val, len != SMIA_REG_32BIT) return -EINVAL; - if (smiapp_quirk_reg(sensor, reg, val)) - goto found_quirk; - if (len == SMIA_REG_8BIT !only8) rval = smiapp_read(sensor, (u16)reg, len, val); else @@ -182,7 +179,6 @@ static int __smiapp_read(struct smiapp_sensor *sensor, u32 reg, u32 *val, if (rval 0) return rval; -found_quirk: if (reg SMIA_REG_FLAG_FLOAT) *val = float_to_u32_mul_100(client, *val); diff --git a/drivers/media/i2c/smiapp/smiapp-regs.h b/drivers/media/i2c/smiapp/smiapp-regs.h index eefc6c8..e07b30c 100644 --- a/drivers/media/i2c/smiapp/smiapp-regs.h +++ b/drivers/media/i2c/smiapp/smiapp-regs.h @@ -34,11 +34,6 @@ #define SMIA_REG_8BIT 1 #define SMIA_REG_16BIT 2 #define SMIA_REG_32BIT 4 -struct smia_reg { - u16 type; - u16 reg;/* 16-bit offset */ - u32 val;/* 8/16/32-bit value */ -}; struct smiapp_sensor; -- 1.8.3.2 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More
[PATCH v2 06/21] smiapp: Make PLL flags separate from regular quirk flags
It doesn't make sense to just copy the information to the PLL flags. Add a new fields for the quirks to contain the PLL flags. Signed-off-by: Sakari Ailus sakari.ai...@linux.intel.com --- drivers/media/i2c/smiapp/smiapp-core.c | 5 ++--- drivers/media/i2c/smiapp/smiapp-quirk.c | 2 +- drivers/media/i2c/smiapp/smiapp-quirk.h | 5 ++--- 3 files changed, 5 insertions(+), 7 deletions(-) diff --git a/drivers/media/i2c/smiapp/smiapp-core.c b/drivers/media/i2c/smiapp/smiapp-core.c index 69c11ec..23f2c4d 100644 --- a/drivers/media/i2c/smiapp/smiapp-core.c +++ b/drivers/media/i2c/smiapp/smiapp-core.c @@ -2617,12 +2617,11 @@ static int smiapp_registered(struct v4l2_subdev *subdev) pll-bus_type = SMIAPP_PLL_BUS_TYPE_CSI2; pll-csi2.lanes = sensor-platform_data-lanes; pll-ext_clk_freq_hz = sensor-platform_data-ext_clk; + if (sensor-minfo.quirk) + pll-flags = sensor-minfo.quirk-pll_flags; /* Profile 0 sensors have no separate OP clock branch. */ if (sensor-minfo.smiapp_profile == SMIAPP_PROFILE_0) pll-flags |= SMIAPP_PLL_FLAG_NO_OP_CLOCKS; - if (smiapp_needs_quirk(sensor, - SMIAPP_QUIRK_FLAG_OP_PIX_CLOCK_PER_LANE)) - pll-flags |= SMIAPP_PLL_FLAG_OP_PIX_CLOCK_PER_LANE; pll-scale_n = sensor-limits[SMIAPP_LIMIT_SCALER_N_MIN]; rval = smiapp_update_mode(sensor); diff --git a/drivers/media/i2c/smiapp/smiapp-quirk.c b/drivers/media/i2c/smiapp/smiapp-quirk.c index 06a0c21..bd2f8a7 100644 --- a/drivers/media/i2c/smiapp/smiapp-quirk.c +++ b/drivers/media/i2c/smiapp/smiapp-quirk.c @@ -225,7 +225,7 @@ const struct smiapp_quirk smiapp_jt8ev1_quirk = { .post_poweron = jt8ev1_post_poweron, .pre_streamon = jt8ev1_pre_streamon, .post_streamoff = jt8ev1_post_streamoff, - .flags = SMIAPP_QUIRK_FLAG_OP_PIX_CLOCK_PER_LANE, + .pll_flags = SMIAPP_PLL_FLAG_OP_PIX_CLOCK_PER_LANE, }; static int tcm8500md_limits(struct smiapp_sensor *sensor) diff --git a/drivers/media/i2c/smiapp/smiapp-quirk.h b/drivers/media/i2c/smiapp/smiapp-quirk.h index 96a253e..ea8231c6 100644 --- a/drivers/media/i2c/smiapp/smiapp-quirk.h +++ b/drivers/media/i2c/smiapp/smiapp-quirk.h @@ -42,11 +42,10 @@ struct smiapp_quirk { int (*pre_streamon)(struct smiapp_sensor *sensor); int (*post_streamoff)(struct smiapp_sensor *sensor); unsigned long flags; + unsigned long pll_flags; }; -/* op pix clock is for all lanes in total normally */ -#define SMIAPP_QUIRK_FLAG_OP_PIX_CLOCK_PER_LANE(1 0) -#define SMIAPP_QUIRK_FLAG_8BIT_READ_ONLY (1 1) +#define SMIAPP_QUIRK_FLAG_8BIT_READ_ONLY (1 0) struct smiapp_reg_8 { u16 reg; -- 1.8.3.2 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 17/21] smiapp-pll: Add pixel rate in pixel array as output parameters
The actual pixel array pixel rate may be something else than vt_pix_clk_freq on some implementations. Add a new field which contains the corrected value. Signed-off-by: Sakari Ailus sakari.ai...@linux.intel.com --- drivers/media/i2c/smiapp-pll.c | 1 + drivers/media/i2c/smiapp-pll.h | 1 + 2 files changed, 2 insertions(+) diff --git a/drivers/media/i2c/smiapp-pll.c b/drivers/media/i2c/smiapp-pll.c index aca0ed7..25abf01 100644 --- a/drivers/media/i2c/smiapp-pll.c +++ b/drivers/media/i2c/smiapp-pll.c @@ -335,6 +335,7 @@ static int __smiapp_pll_calculate(struct device *dev, pll-pixel_rate_csi = pll-op_pix_clk_freq_hz * lane_op_clock_ratio; + pll-pixel_rate_pixel_array = pll-vt_pix_clk_freq_hz; rval = bounds_check(dev, pll-pll_ip_clk_freq_hz, limits-min_pll_ip_freq_hz, diff --git a/drivers/media/i2c/smiapp-pll.h b/drivers/media/i2c/smiapp-pll.h index 02d11db..c6ad809 100644 --- a/drivers/media/i2c/smiapp-pll.h +++ b/drivers/media/i2c/smiapp-pll.h @@ -75,6 +75,7 @@ struct smiapp_pll { uint32_t vt_pix_clk_freq_hz; uint32_t pixel_rate_csi; + uint32_t pixel_rate_pixel_array; }; struct smiapp_pll_branch_limits { -- 1.8.3.2 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 14/21] smiapp-pll: Add support for odd pre-pll divisors
Some sensors support odd pre-pll divisor. Signed-off-by: Sakari Ailus sakari.ai...@linux.intel.com --- drivers/media/i2c/smiapp-pll.c | 39 ++- drivers/media/i2c/smiapp-pll.h | 2 ++ 2 files changed, 28 insertions(+), 13 deletions(-) diff --git a/drivers/media/i2c/smiapp-pll.c b/drivers/media/i2c/smiapp-pll.c index ec9f8bb..bed44c0 100644 --- a/drivers/media/i2c/smiapp-pll.c +++ b/drivers/media/i2c/smiapp-pll.c @@ -34,14 +34,18 @@ static inline uint64_t div_u64_round_up(uint64_t dividend, uint32_t divisor) } /* Return an even number or one. */ -static inline uint32_t clk_div_even(uint32_t a) +static inline uint32_t clk_div_even(uint32_t a, bool allow_odd) { + if (allow_odd) + return a; return max_t(uint32_t, 1, a ~1); } /* Return an even number or one. */ -static inline uint32_t clk_div_even_up(uint32_t a) +static inline uint32_t clk_div_even_up(uint32_t a, bool allow_odd) { + if (allow_odd) + return a; if (a == 1) return 1; return (a + 1) ~1; @@ -269,13 +273,13 @@ static int __smiapp_pll_calculate(struct device *dev, min_sys_div = max(min_sys_div, DIV_ROUND_UP(min_vt_div, limits-vt.max_pix_clk_div)); - dev_dbg(dev, min_sys_div: max_vt_pix_clk_div: %u\n, min_sys_div); + dev_dbg(dev, min_sys_div: max_vt_pix_clk_div: %d\n, min_sys_div); min_sys_div = max_t(uint32_t, min_sys_div, pll-pll_op_clk_freq_hz / limits-vt.max_sys_clk_freq_hz); - dev_dbg(dev, min_sys_div: max_pll_op_clk_freq_hz: %u\n, min_sys_div); - min_sys_div = clk_div_even_up(min_sys_div); - dev_dbg(dev, min_sys_div: one or even: %u\n, min_sys_div); + dev_dbg(dev, min_sys_div: max_pll_op_clk_freq_hz: %d\n, min_sys_div); + min_sys_div = clk_div_even_up(min_sys_div, 0); + dev_dbg(dev, min_sys_div: one or even: %d\n, min_sys_div); max_sys_div = limits-vt.max_sys_clk_div; dev_dbg(dev, max_sys_div: %u\n, max_sys_div); @@ -423,14 +427,19 @@ int smiapp_pll_calculate(struct device *dev, limits-min_pre_pll_clk_div, limits-max_pre_pll_clk_div); max_pre_pll_clk_div = min_t(uint16_t, limits-max_pre_pll_clk_div, - clk_div_even(pll-ext_clk_freq_hz / - limits-min_pll_ip_freq_hz)); + clk_div_even( + pll-ext_clk_freq_hz / + limits-min_pll_ip_freq_hz, + pll-flags + SMIAPP_PLL_FLAG_ALLOW_ODD_PRE_PLL_CLK_DIV)); min_pre_pll_clk_div = max_t(uint16_t, limits-min_pre_pll_clk_div, clk_div_even_up( DIV_ROUND_UP(pll-ext_clk_freq_hz, - limits-max_pll_ip_freq_hz))); - dev_dbg(dev, pre-pll check: min / max pre_pll_clk_div: %u / %u\n, + limits-max_pll_ip_freq_hz), + pll-flags + SMIAPP_PLL_FLAG_ALLOW_ODD_PRE_PLL_CLK_DIV)); + dev_dbg(dev, pre-pll check: min / max pre_pll_clk_div: %d / %d\n, min_pre_pll_clk_div, max_pre_pll_clk_div); i = gcd(pll-pll_op_clk_freq_hz, pll-ext_clk_freq_hz); @@ -442,13 +451,17 @@ int smiapp_pll_calculate(struct device *dev, max_t(uint16_t, min_pre_pll_clk_div, clk_div_even_up( DIV_ROUND_UP(mul * pll-ext_clk_freq_hz, - limits-max_pll_op_freq_hz))); - dev_dbg(dev, pll_op check: min / max pre_pll_clk_div: %u / %u\n, + limits-max_pll_op_freq_hz), + pll-flags + SMIAPP_PLL_FLAG_ALLOW_ODD_PRE_PLL_CLK_DIV)); + dev_dbg(dev, pll_op check: min / max pre_pll_clk_div: %d / %d\n, min_pre_pll_clk_div, max_pre_pll_clk_div); for (pll-pre_pll_clk_div = min_pre_pll_clk_div; pll-pre_pll_clk_div = max_pre_pll_clk_div; -pll-pre_pll_clk_div += 2 - (pll-pre_pll_clk_div 1)) { +pll-pre_pll_clk_div += +pll-flags SMIAPP_PLL_FLAG_ALLOW_ODD_PRE_PLL_CLK_DIV +? 1 : (2 - (pll-pre_pll_clk_div 1))) { rval = __smiapp_pll_calculate(dev, limits, pll, mul, div, lane_op_clock_ratio); if (rval) diff --git a/drivers/media/i2c/smiapp-pll.h b/drivers/media/i2c/smiapp-pll.h index bb5ae28..a25f550 100644 --- a/drivers/media/i2c/smiapp-pll.h +++ b/drivers/media/i2c/smiapp-pll.h @@ -34,6 +34,8 @@ /* op pix clock is for all lanes in total normally */ #define SMIAPP_PLL_FLAG_OP_PIX_CLOCK_PER_LANE
[PATCH v2 07/21] smiapp: Make PLL flags unsigned long
No reason to keep this u8, really. Signed-off-by: Sakari Ailus sakari.ai...@linux.intel.com --- drivers/media/i2c/smiapp-pll.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/i2c/smiapp-pll.h b/drivers/media/i2c/smiapp-pll.h index a4a6498..5ce2b61 100644 --- a/drivers/media/i2c/smiapp-pll.h +++ b/drivers/media/i2c/smiapp-pll.h @@ -46,7 +46,7 @@ struct smiapp_pll { uint8_t bus_width; } parallel; }; - uint8_t flags; + unsigned long flags; uint8_t binning_horizontal; uint8_t binning_vertical; uint8_t scale_m; -- 1.8.3.2 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 18/21] smiapp: Use actual pixel rate calculated by the PLL calculator
Signed-off-by: Sakari Ailus sakari.ai...@linux.intel.com --- drivers/media/i2c/smiapp/smiapp-core.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/media/i2c/smiapp/smiapp-core.c b/drivers/media/i2c/smiapp/smiapp-core.c index 6d940f0..284df17 100644 --- a/drivers/media/i2c/smiapp/smiapp-core.c +++ b/drivers/media/i2c/smiapp/smiapp-core.c @@ -297,7 +297,7 @@ static int smiapp_pll_update(struct smiapp_sensor *sensor) if (rval 0) return rval; - sensor-pixel_rate_parray-cur.val64 = pll-vt_pix_clk_freq_hz; + sensor-pixel_rate_parray-cur.val64 = pll-pixel_rate_pixel_array; sensor-pixel_rate_csi-cur.val64 = pll-pixel_rate_csi; return 0; @@ -865,7 +865,7 @@ static int smiapp_update_mode(struct smiapp_sensor *sensor) dev_dbg(client-dev, hblank\t\t%d\n, sensor-hblank-val); dev_dbg(client-dev, real timeperframe\t100/%d\n, - sensor-pll.vt_pix_clk_freq_hz / + sensor-pll.pixel_rate_pixel_array / ((sensor-pixel_array-crop[SMIAPP_PA_PAD_SRC].width + sensor-hblank-val) * (sensor-pixel_array-crop[SMIAPP_PA_PAD_SRC].height -- 1.8.3.2 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 05/21] smiapp: Use I2C adapter ID and address in the sub-device name
The sub-device names should be unique. Should two identical sensors be present in the same media device they would be indistinguishable. The names will change e.g. from vs6555 pixel array to vs6555 1-0010 pixel array. Signed-off-by: Sakari Ailus sakari.ai...@linux.intel.com --- drivers/media/i2c/smiapp/smiapp-core.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/media/i2c/smiapp/smiapp-core.c b/drivers/media/i2c/smiapp/smiapp-core.c index 8741cae..69c11ec 100644 --- a/drivers/media/i2c/smiapp/smiapp-core.c +++ b/drivers/media/i2c/smiapp/smiapp-core.c @@ -2543,8 +2543,9 @@ static int smiapp_registered(struct v4l2_subdev *subdev) } snprintf(this-sd.name, -sizeof(this-sd.name), %s %s, -sensor-minfo.name, _this-name); +sizeof(this-sd.name), %s %d-%4.4x %s, +sensor-minfo.name, i2c_adapter_id(client-adapter), +client-addr, _this-name); this-sink_fmt.width = sensor-limits[SMIAPP_LIMIT_X_ADDR_MAX] + 1; -- 1.8.3.2 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 19/21] smiapp-pll: Add quirk flag for sensors that effectively use double pix clks
Some sensors have effectively the double pixel (and other clocks) compared to calculations. The frequency of the bus is also affected similarly so take this into account when calculating pll_op_clock frequency. Signed-off-by: Sakari Ailus sakari.ai...@linux.intel.com --- drivers/media/i2c/smiapp-pll.c | 10 ++ drivers/media/i2c/smiapp-pll.h | 6 ++ 2 files changed, 16 insertions(+) diff --git a/drivers/media/i2c/smiapp-pll.c b/drivers/media/i2c/smiapp-pll.c index 25abf01..7d45815 100644 --- a/drivers/media/i2c/smiapp-pll.c +++ b/drivers/media/i2c/smiapp-pll.c @@ -336,6 +336,10 @@ static int __smiapp_pll_calculate(struct device *dev, pll-pixel_rate_csi = pll-op_pix_clk_freq_hz * lane_op_clock_ratio; pll-pixel_rate_pixel_array = pll-vt_pix_clk_freq_hz; + if (pll-flags SMIAPP_PLL_FLAG_PIX_CLOCK_DOUBLE) { + pll-pixel_rate_csi *= 2; + pll-pixel_rate_pixel_array *= 2; + } rval = bounds_check(dev, pll-pll_ip_clk_freq_hz, limits-min_pll_ip_freq_hz, @@ -427,6 +431,12 @@ int smiapp_pll_calculate(struct device *dev, */ if (pll-flags SMIAPP_PLL_FLAG_OP_PIX_DIV_HALF) pll-pll_op_clk_freq_hz /= 2; + /* +* If it'll be multiplied by two in the end divide it now to +* avoid achieving double the desired clock. +*/ + if (pll-flags SMIAPP_PLL_FLAG_PIX_CLOCK_DOUBLE) + pll-pll_op_clk_freq_hz /= 2; /* Figure out limits for pre-pll divider based on extclk */ dev_dbg(dev, min / max pre_pll_clk_div: %u / %u\n, diff --git a/drivers/media/i2c/smiapp-pll.h b/drivers/media/i2c/smiapp-pll.h index c6ad809..9eaac54 100644 --- a/drivers/media/i2c/smiapp-pll.h +++ b/drivers/media/i2c/smiapp-pll.h @@ -38,6 +38,12 @@ #define SMIAPP_PLL_FLAG_ALLOW_ODD_PRE_PLL_CLK_DIV (1 2) /* op pix div value is half of the bits-per-pixel value */ #define SMIAPP_PLL_FLAG_OP_PIX_DIV_HALF(1 3) +/* + * The effective vt and op pix clocks are twice as high as the + * calculated value. The limits are still against the regular limit + * values. + */ +#define SMIAPP_PLL_FLAG_PIX_CLOCK_DOUBLE (1 4) struct smiapp_pll { /* input values */ -- 1.8.3.2 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 11/21] smiapp-pll: The clock tree values are unsigned --- fix debug prints
These values are unsigned, so use %u instead of %d. Signed-off-by: Sakari Ailus sakari.ai...@linux.intel.com --- drivers/media/i2c/smiapp-pll.c | 94 +- 1 file changed, 47 insertions(+), 47 deletions(-) diff --git a/drivers/media/i2c/smiapp-pll.c b/drivers/media/i2c/smiapp-pll.c index ab5d9a3..d14af5c 100644 --- a/drivers/media/i2c/smiapp-pll.c +++ b/drivers/media/i2c/smiapp-pll.c @@ -65,26 +65,26 @@ static int bounds_check(struct device *dev, uint32_t val, static void print_pll(struct device *dev, struct smiapp_pll *pll) { - dev_dbg(dev, pre_pll_clk_div\t%d\n, pll-pre_pll_clk_div); - dev_dbg(dev, pll_multiplier \t%d\n, pll-pll_multiplier); + dev_dbg(dev, pre_pll_clk_div\t%u\n, pll-pre_pll_clk_div); + dev_dbg(dev, pll_multiplier \t%u\n, pll-pll_multiplier); if (!(pll-flags SMIAPP_PLL_FLAG_NO_OP_CLOCKS)) { - dev_dbg(dev, op_sys_clk_div \t%d\n, pll-op_sys_clk_div); - dev_dbg(dev, op_pix_clk_div \t%d\n, pll-op_pix_clk_div); + dev_dbg(dev, op_sys_clk_div \t%u\n, pll-op_sys_clk_div); + dev_dbg(dev, op_pix_clk_div \t%u\n, pll-op_pix_clk_div); } - dev_dbg(dev, vt_sys_clk_div \t%d\n, pll-vt_sys_clk_div); - dev_dbg(dev, vt_pix_clk_div \t%d\n, pll-vt_pix_clk_div); + dev_dbg(dev, vt_sys_clk_div \t%u\n, pll-vt_sys_clk_div); + dev_dbg(dev, vt_pix_clk_div \t%u\n, pll-vt_pix_clk_div); - dev_dbg(dev, ext_clk_freq_hz \t%d\n, pll-ext_clk_freq_hz); - dev_dbg(dev, pll_ip_clk_freq_hz \t%d\n, pll-pll_ip_clk_freq_hz); - dev_dbg(dev, pll_op_clk_freq_hz \t%d\n, pll-pll_op_clk_freq_hz); + dev_dbg(dev, ext_clk_freq_hz \t%u\n, pll-ext_clk_freq_hz); + dev_dbg(dev, pll_ip_clk_freq_hz \t%u\n, pll-pll_ip_clk_freq_hz); + dev_dbg(dev, pll_op_clk_freq_hz \t%u\n, pll-pll_op_clk_freq_hz); if (!(pll-flags SMIAPP_PLL_FLAG_NO_OP_CLOCKS)) { - dev_dbg(dev, op_sys_clk_freq_hz \t%d\n, + dev_dbg(dev, op_sys_clk_freq_hz \t%u\n, pll-op_sys_clk_freq_hz); - dev_dbg(dev, op_pix_clk_freq_hz \t%d\n, + dev_dbg(dev, op_pix_clk_freq_hz \t%u\n, pll-op_pix_clk_freq_hz); } - dev_dbg(dev, vt_sys_clk_freq_hz \t%d\n, pll-vt_sys_clk_freq_hz); - dev_dbg(dev, vt_pix_clk_freq_hz \t%d\n, pll-vt_pix_clk_freq_hz); + dev_dbg(dev, vt_sys_clk_freq_hz \t%u\n, pll-vt_sys_clk_freq_hz); + dev_dbg(dev, vt_pix_clk_freq_hz \t%u\n, pll-vt_pix_clk_freq_hz); } /* @@ -123,11 +123,11 @@ static int __smiapp_pll_calculate(struct device *dev, * Get pre_pll_clk_div so that our pll_op_clk_freq_hz won't be * too high. */ - dev_dbg(dev, pre_pll_clk_div %d\n, pll-pre_pll_clk_div); + dev_dbg(dev, pre_pll_clk_div %u\n, pll-pre_pll_clk_div); /* Don't go above max pll multiplier. */ more_mul_max = limits-max_pll_multiplier / mul; - dev_dbg(dev, more_mul_max: max_pll_multiplier check: %d\n, + dev_dbg(dev, more_mul_max: max_pll_multiplier check: %u\n, more_mul_max); /* Don't go above max pll op frequency. */ more_mul_max = @@ -135,30 +135,30 @@ static int __smiapp_pll_calculate(struct device *dev, more_mul_max, limits-max_pll_op_freq_hz / (pll-ext_clk_freq_hz / pll-pre_pll_clk_div * mul)); - dev_dbg(dev, more_mul_max: max_pll_op_freq_hz check: %d\n, + dev_dbg(dev, more_mul_max: max_pll_op_freq_hz check: %u\n, more_mul_max); /* Don't go above the division capability of op sys clock divider. */ more_mul_max = min(more_mul_max, limits-op.max_sys_clk_div * pll-pre_pll_clk_div / div); - dev_dbg(dev, more_mul_max: max_op_sys_clk_div check: %d\n, + dev_dbg(dev, more_mul_max: max_op_sys_clk_div check: %u\n, more_mul_max); /* Ensure we won't go above min_pll_multiplier. */ more_mul_max = min(more_mul_max, DIV_ROUND_UP(limits-max_pll_multiplier, mul)); - dev_dbg(dev, more_mul_max: min_pll_multiplier check: %d\n, + dev_dbg(dev, more_mul_max: min_pll_multiplier check: %u\n, more_mul_max); /* Ensure we won't go below min_pll_op_freq_hz. */ more_mul_min = DIV_ROUND_UP(limits-min_pll_op_freq_hz, pll-ext_clk_freq_hz / pll-pre_pll_clk_div * mul); - dev_dbg(dev, more_mul_min: min_pll_op_freq_hz check: %d\n, + dev_dbg(dev, more_mul_min: min_pll_op_freq_hz check: %u\n, more_mul_min); /* Ensure we won't go below min_pll_multiplier. */ more_mul_min = max(more_mul_min, DIV_ROUND_UP(limits-min_pll_multiplier, mul)); - dev_dbg(dev, more_mul_min:
[PATCH v2 20/21] smiapp: Define macros for obtaining properties of register definitions
The register address, width and flags are encoded as a 32-bit value. Add macros for obtaining these separately. Use the macros in register access functions. Signed-off-by: Sakari Ailus sakari.ai...@linux.intel.com --- drivers/media/i2c/smiapp/smiapp-regs.c | 13 +++-- drivers/media/i2c/smiapp/smiapp-regs.h | 4 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/drivers/media/i2c/smiapp/smiapp-regs.c b/drivers/media/i2c/smiapp/smiapp-regs.c index c2db205..47b9e4c 100644 --- a/drivers/media/i2c/smiapp/smiapp-regs.c +++ b/drivers/media/i2c/smiapp/smiapp-regs.c @@ -165,7 +165,7 @@ static int __smiapp_read(struct smiapp_sensor *sensor, u32 reg, u32 *val, bool only8) { struct i2c_client *client = v4l2_get_subdevdata(sensor-src-sd); - unsigned int len = (u8)(reg 16); + u8 len = SMIAPP_REG_WIDTH(reg); int rval; if (len != SMIAPP_REG_8BIT len != SMIAPP_REG_16BIT @@ -173,9 +173,10 @@ static int __smiapp_read(struct smiapp_sensor *sensor, u32 reg, u32 *val, return -EINVAL; if (len == SMIAPP_REG_8BIT || !only8) - rval = smiapp_read(sensor, (u16)reg, len, val); + rval = smiapp_read(sensor, SMIAPP_REG_ADDR(reg), len, val); else - rval = smiapp_read_8only(sensor, (u16)reg, len, val); + rval = smiapp_read_8only(sensor, SMIAPP_REG_ADDR(reg), len, +val); if (rval 0) return rval; @@ -208,9 +209,9 @@ int smiapp_write(struct smiapp_sensor *sensor, u32 reg, u32 val) struct i2c_msg msg; unsigned char data[6]; unsigned int retries; - unsigned int flags = reg 24; - unsigned int len = (u8)(reg 16); - u16 offset = reg; + u8 flags = SMIAPP_REG_FLAGS(reg); + u8 len = SMIAPP_REG_WIDTH(reg); + u16 offset = SMIAPP_REG_ADDR(reg); int r; if ((len != SMIAPP_REG_8BIT len != SMIAPP_REG_16BIT diff --git a/drivers/media/i2c/smiapp/smiapp-regs.h b/drivers/media/i2c/smiapp/smiapp-regs.h index 934130b..aeecab8 100644 --- a/drivers/media/i2c/smiapp/smiapp-regs.h +++ b/drivers/media/i2c/smiapp/smiapp-regs.h @@ -28,6 +28,10 @@ #include linux/i2c.h #include linux/types.h +#define SMIAPP_REG_ADDR(reg) ((u16)reg) +#define SMIAPP_REG_WIDTH(reg) ((u8)(reg 16)) +#define SMIAPP_REG_FLAGS(reg) ((u8)(reg 24)) + /* Use upper 8 bits of the type field for flags */ #define SMIAPP_REG_FLAG_FLOAT (1 24) -- 1.8.3.2 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 21/21] smiapp: Add register diversion quirk
Add a quirk for diverting registers for on some sensors, even the standard registers are not where they can be expected to be found. Add a quirk to to help using such sensors. smiapp_write_no_quirk() and smiapp_read_no_quirk() functions are provided for the use of quirk implementations. Signed-off-by: Sakari Ailus sakari.ai...@linux.intel.com --- drivers/media/i2c/smiapp/smiapp-quirk.h | 13 + drivers/media/i2c/smiapp/smiapp-regs.c | 48 - drivers/media/i2c/smiapp/smiapp-regs.h | 2 ++ 3 files changed, 57 insertions(+), 6 deletions(-) diff --git a/drivers/media/i2c/smiapp/smiapp-quirk.h b/drivers/media/i2c/smiapp/smiapp-quirk.h index b8b4087..ddc0548 100644 --- a/drivers/media/i2c/smiapp/smiapp-quirk.h +++ b/drivers/media/i2c/smiapp/smiapp-quirk.h @@ -35,6 +35,17 @@ struct smiapp_sensor; * @post_poweron: Called always after the sensor has been fully powered on. * @pre_streamon: Called just before streaming is enabled. * @post_streamon: Called right after stopping streaming. + * @reg_access: Register access quirk. The quirk may divert the access + * to another register, or no register at all. + * + * @write: Is this read (false) or write (true) access? + * @reg: Pointer to the register to access + * @value: Register value, set by the caller on write, or + * by the quirk on read + * + * @return: 0 on success, -ENOIOCTLCMD if no register + * access may be done by the caller (default read + * value is zero), else negative error code on error */ struct smiapp_quirk { int (*limits)(struct smiapp_sensor *sensor); @@ -42,6 +53,8 @@ struct smiapp_quirk { int (*pre_streamon)(struct smiapp_sensor *sensor); int (*post_streamoff)(struct smiapp_sensor *sensor); unsigned long (*pll_flags)(struct smiapp_sensor *sensor); + int (*reg_access)(struct smiapp_sensor *sensor, bool write, u32 *reg, + u32 *val); unsigned long flags; }; diff --git a/drivers/media/i2c/smiapp/smiapp-regs.c b/drivers/media/i2c/smiapp/smiapp-regs.c index 47b9e4c..a209800 100644 --- a/drivers/media/i2c/smiapp/smiapp-regs.c +++ b/drivers/media/i2c/smiapp/smiapp-regs.c @@ -186,7 +186,7 @@ static int __smiapp_read(struct smiapp_sensor *sensor, u32 reg, u32 *val, return 0; } -int smiapp_read(struct smiapp_sensor *sensor, u32 reg, u32 *val) +int smiapp_read_no_quirk(struct smiapp_sensor *sensor, u32 reg, u32 *val) { return __smiapp_read( sensor, reg, val, @@ -194,16 +194,35 @@ int smiapp_read(struct smiapp_sensor *sensor, u32 reg, u32 *val) SMIAPP_QUIRK_FLAG_8BIT_READ_ONLY)); } +int smiapp_read(struct smiapp_sensor *sensor, u32 reg, u32 *val) +{ + int rval; + + *val = 0; + rval = smiapp_call_quirk(sensor, reg_access, false, reg, val); + if (rval == -ENOIOCTLCMD) + return 0; + if (rval 0) + return rval; + + return smiapp_read_no_quirk(sensor, reg, val); +} + int smiapp_read_8only(struct smiapp_sensor *sensor, u32 reg, u32 *val) { + int rval; + + *val = 0; + rval = smiapp_call_quirk(sensor, reg_access, false, reg, val); + if (rval == -ENOIOCTLCMD) + return 0; + if (rval 0) + return rval; + return __smiapp_read(sensor, reg, val, true); } -/* - * Write to a 8/16-bit register. - * Returns zero if successful, or non-zero otherwise. - */ -int smiapp_write(struct smiapp_sensor *sensor, u32 reg, u32 val) +int smiapp_write_no_quirk(struct smiapp_sensor *sensor, u32 reg, u32 val) { struct i2c_client *client = v4l2_get_subdevdata(sensor-src-sd); struct i2c_msg msg; @@ -268,3 +287,20 @@ int smiapp_write(struct smiapp_sensor *sensor, u32 reg, u32 val) return r; } + +/* + * Write to a 8/16-bit register. + * Returns zero if successful, or non-zero otherwise. + */ +int smiapp_write(struct smiapp_sensor *sensor, u32 reg, u32 val) +{ + int rval; + + rval = smiapp_call_quirk(sensor, reg_access, true, reg, val); + if (rval == -ENOIOCTLCMD) + return 0; + if (rval 0) + return rval; + + return smiapp_write_no_quirk(sensor, reg, val); +} diff --git a/drivers/media/i2c/smiapp/smiapp-regs.h b/drivers/media/i2c/smiapp/smiapp-regs.h index aeecab8..3552112 100644 --- a/drivers/media/i2c/smiapp/smiapp-regs.h +++ b/drivers/media/i2c/smiapp/smiapp-regs.h @@ -41,8 +41,10 @@ struct smiapp_sensor; +int smiapp_read_no_quirk(struct smiapp_sensor *sensor, u32 reg, u32 *val); int smiapp_read(struct smiapp_sensor *sensor, u32 reg, u32 *val); int smiapp_read_8only(struct smiapp_sensor *sensor, u32 reg, u32 *val); +int smiapp_write_no_quirk(struct smiapp_sensor *sensor, u32 reg, u32 val); int smiapp_write(struct smiapp_sensor *sensor, u32 reg, u32
[PATCH v2 12/21] smiapp: Limits can be 64 bits
Limits may exceed the value range of 32 bit unsigned integers. Thus use 64 bits instead. Use typed min/max/clamp macros. Debug printing changes as well. Signed-off-by: Sakari Ailus sakari.ai...@linux.intel.com --- drivers/media/i2c/smiapp/smiapp-core.c | 30 -- drivers/media/i2c/smiapp/smiapp-quirk.c | 4 ++-- drivers/media/i2c/smiapp/smiapp-quirk.h | 2 +- drivers/media/i2c/smiapp/smiapp.h | 2 +- 4 files changed, 20 insertions(+), 18 deletions(-) diff --git a/drivers/media/i2c/smiapp/smiapp-core.c b/drivers/media/i2c/smiapp/smiapp-core.c index 3af8df8..6d940f0 100644 --- a/drivers/media/i2c/smiapp/smiapp-core.c +++ b/drivers/media/i2c/smiapp/smiapp-core.c @@ -502,7 +502,8 @@ static int smiapp_init_controls(struct smiapp_sensor *sensor) V4L2_CID_ANALOGUE_GAIN, sensor-limits[SMIAPP_LIMIT_ANALOGUE_GAIN_CODE_MIN], sensor-limits[SMIAPP_LIMIT_ANALOGUE_GAIN_CODE_MAX], - max(sensor-limits[SMIAPP_LIMIT_ANALOGUE_GAIN_CODE_STEP], 1U), + max_t(uint32_t, + sensor-limits[SMIAPP_LIMIT_ANALOGUE_GAIN_CODE_STEP], 1U), sensor-limits[SMIAPP_LIMIT_ANALOGUE_GAIN_CODE_MIN]); /* Exposure limits will be updated soon, use just something here. */ @@ -679,7 +680,7 @@ static int smiapp_get_limits_binning(struct smiapp_sensor *sensor) for (i = 0; i ARRAY_SIZE(limits); i++) { dev_dbg(client-dev, - replace limit 0x%8.8x \%s\ = %d, 0x%x\n, + replace limit 0x%8.8x \%s\ = %llu, 0x%llx\n, smiapp_reg_limits[limits[i]].addr, smiapp_reg_limits[limits[i]].what, sensor-limits[limits_replace[i]], @@ -1689,13 +1690,13 @@ static int smiapp_set_format(struct v4l2_subdev *subdev, fmt-format.height = ~1; fmt-format.width = - clamp(fmt-format.width, - sensor-limits[SMIAPP_LIMIT_MIN_X_OUTPUT_SIZE], - sensor-limits[SMIAPP_LIMIT_MAX_X_OUTPUT_SIZE]); + clamp_t(uint32_t, fmt-format.width, + sensor-limits[SMIAPP_LIMIT_MIN_X_OUTPUT_SIZE], + sensor-limits[SMIAPP_LIMIT_MAX_X_OUTPUT_SIZE]); fmt-format.height = - clamp(fmt-format.height, - sensor-limits[SMIAPP_LIMIT_MIN_Y_OUTPUT_SIZE], - sensor-limits[SMIAPP_LIMIT_MAX_Y_OUTPUT_SIZE]); + clamp_t(uint32_t, fmt-format.height, + sensor-limits[SMIAPP_LIMIT_MIN_Y_OUTPUT_SIZE], + sensor-limits[SMIAPP_LIMIT_MAX_Y_OUTPUT_SIZE]); smiapp_get_crop_compose(subdev, fh, crops, NULL, fmt-which); @@ -1834,12 +1835,13 @@ static void smiapp_set_compose_scaler(struct v4l2_subdev *subdev, * sensor-limits[SMIAPP_LIMIT_SCALER_N_MIN] / sensor-limits[SMIAPP_LIMIT_MIN_X_OUTPUT_SIZE]; - a = clamp(a, sensor-limits[SMIAPP_LIMIT_SCALER_M_MIN], - sensor-limits[SMIAPP_LIMIT_SCALER_M_MAX]); - b = clamp(b, sensor-limits[SMIAPP_LIMIT_SCALER_M_MIN], - sensor-limits[SMIAPP_LIMIT_SCALER_M_MAX]); - max_m = clamp(max_m, sensor-limits[SMIAPP_LIMIT_SCALER_M_MIN], - sensor-limits[SMIAPP_LIMIT_SCALER_M_MAX]); + a = clamp_t(uint32_t, a, sensor-limits[SMIAPP_LIMIT_SCALER_M_MIN], + sensor-limits[SMIAPP_LIMIT_SCALER_M_MAX]); + b = clamp_t(uint32_t, b, sensor-limits[SMIAPP_LIMIT_SCALER_M_MIN], + sensor-limits[SMIAPP_LIMIT_SCALER_M_MAX]); + max_m = clamp_t(uint32_t, max_m, + sensor-limits[SMIAPP_LIMIT_SCALER_M_MIN], + sensor-limits[SMIAPP_LIMIT_SCALER_M_MAX]); dev_dbg(client-dev, scaling: a %d b %d max_m %d\n, a, b, max_m); diff --git a/drivers/media/i2c/smiapp/smiapp-quirk.c b/drivers/media/i2c/smiapp/smiapp-quirk.c index e0bee87..108ea23 100644 --- a/drivers/media/i2c/smiapp/smiapp-quirk.c +++ b/drivers/media/i2c/smiapp/smiapp-quirk.c @@ -51,11 +51,11 @@ static int smiapp_write_8s(struct smiapp_sensor *sensor, } void smiapp_replace_limit(struct smiapp_sensor *sensor, - u32 limit, u32 val) + u32 limit, u64 val) { struct i2c_client *client = v4l2_get_subdevdata(sensor-src-sd); - dev_dbg(client-dev, quirk: 0x%8.8x \%s\ = %d, 0x%x\n, + dev_dbg(client-dev, quirk: 0x%8.8x \%s\ = %llu, 0x%llx\n, smiapp_reg_limits[limit].addr, smiapp_reg_limits[limit].what, val, val); sensor-limits[limit] = val; diff --git a/drivers/media/i2c/smiapp/smiapp-quirk.h b/drivers/media/i2c/smiapp/smiapp-quirk.h index dddb62b..b8b4087 100644 --- a/drivers/media/i2c/smiapp/smiapp-quirk.h +++ b/drivers/media/i2c/smiapp/smiapp-quirk.h @@ -53,7 +53,7 @@ struct smiapp_reg_8 { }; void
[PATCH v2 02/21] smiapp: Rename SMIA_REG to SMIAPP_REG for consistency
SMIAPP_REG_ is the common prefix used in the driver for register related definitions. Use it consistently. Signed-off-by: Sakari Ailus sakari.ai...@linux.intel.com --- drivers/media/i2c/smiapp/smiapp-quirk.c| 2 +- drivers/media/i2c/smiapp/smiapp-reg-defs.h | 8 drivers/media/i2c/smiapp/smiapp-regs.c | 24 drivers/media/i2c/smiapp/smiapp-regs.h | 8 4 files changed, 21 insertions(+), 21 deletions(-) diff --git a/drivers/media/i2c/smiapp/smiapp-quirk.c b/drivers/media/i2c/smiapp/smiapp-quirk.c index 4955289..06a0c21 100644 --- a/drivers/media/i2c/smiapp/smiapp-quirk.c +++ b/drivers/media/i2c/smiapp/smiapp-quirk.c @@ -28,7 +28,7 @@ static int smiapp_write_8(struct smiapp_sensor *sensor, u16 reg, u8 val) { - return smiapp_write(sensor, (SMIA_REG_8BIT 16) | reg, val); + return smiapp_write(sensor, SMIAPP_REG_MK_U8(reg), val); } static int smiapp_write_8s(struct smiapp_sensor *sensor, diff --git a/drivers/media/i2c/smiapp/smiapp-reg-defs.h b/drivers/media/i2c/smiapp/smiapp-reg-defs.h index 3aa0ca9..c488ef0 100644 --- a/drivers/media/i2c/smiapp/smiapp-reg-defs.h +++ b/drivers/media/i2c/smiapp/smiapp-reg-defs.h @@ -21,11 +21,11 @@ * 02110-1301 USA * */ -#define SMIAPP_REG_MK_U8(r) ((SMIA_REG_8BIT 16) | (r)) -#define SMIAPP_REG_MK_U16(r) ((SMIA_REG_16BIT 16) | (r)) -#define SMIAPP_REG_MK_U32(r) ((SMIA_REG_32BIT 16) | (r)) +#define SMIAPP_REG_MK_U8(r) ((SMIAPP_REG_8BIT 16) | (r)) +#define SMIAPP_REG_MK_U16(r) ((SMIAPP_REG_16BIT 16) | (r)) +#define SMIAPP_REG_MK_U32(r) ((SMIAPP_REG_32BIT 16) | (r)) -#define SMIAPP_REG_MK_F32(r) (SMIA_REG_FLAG_FLOAT | (SMIA_REG_32BIT 16) | (r)) +#define SMIAPP_REG_MK_F32(r) (SMIAPP_REG_FLAG_FLOAT | (SMIAPP_REG_32BIT 16) | (r)) #define SMIAPP_REG_U16_MODEL_ID SMIAPP_REG_MK_U16(0x) #define SMIAPP_REG_U8_REVISION_NUMBER_MAJOR SMIAPP_REG_MK_U8(0x0002) diff --git a/drivers/media/i2c/smiapp/smiapp-regs.c b/drivers/media/i2c/smiapp/smiapp-regs.c index e01644c..5d0151a 100644 --- a/drivers/media/i2c/smiapp/smiapp-regs.c +++ b/drivers/media/i2c/smiapp/smiapp-regs.c @@ -114,14 +114,14 @@ static int smiapp_read(struct smiapp_sensor *sensor, u16 reg, *val = 0; /* high byte comes first */ switch (len) { - case SMIA_REG_32BIT: + case SMIAPP_REG_32BIT: *val = (data[0] 24) + (data[1] 16) + (data[2] 8) + data[3]; break; - case SMIA_REG_16BIT: + case SMIAPP_REG_16BIT: *val = (data[0] 8) + data[1]; break; - case SMIA_REG_8BIT: + case SMIAPP_REG_8BIT: *val = data[0]; break; default: @@ -168,18 +168,18 @@ static int __smiapp_read(struct smiapp_sensor *sensor, u32 reg, u32 *val, unsigned int len = (u8)(reg 16); int rval; - if (len != SMIA_REG_8BIT len != SMIA_REG_16BIT -len != SMIA_REG_32BIT) + if (len != SMIAPP_REG_8BIT len != SMIAPP_REG_16BIT +len != SMIAPP_REG_32BIT) return -EINVAL; - if (len == SMIA_REG_8BIT !only8) + if (len == SMIAPP_REG_8BIT !only8) rval = smiapp_read(sensor, (u16)reg, len, val); else rval = smiapp_read_8only(sensor, (u16)reg, len, val); if (rval 0) return rval; - if (reg SMIA_REG_FLAG_FLOAT) + if (reg SMIAPP_REG_FLAG_FLOAT) *val = float_to_u32_mul_100(client, *val); return 0; @@ -213,8 +213,8 @@ int smiapp_write(struct smiapp_sensor *sensor, u32 reg, u32 val) u16 offset = reg; int r; - if ((len != SMIA_REG_8BIT len != SMIA_REG_16BIT -len != SMIA_REG_32BIT) || flags) + if ((len != SMIAPP_REG_8BIT len != SMIAPP_REG_16BIT +len != SMIAPP_REG_32BIT) || flags) return -EINVAL; msg.addr = client-addr; @@ -227,14 +227,14 @@ int smiapp_write(struct smiapp_sensor *sensor, u32 reg, u32 val) data[1] = (u8) (reg 0xff); switch (len) { - case SMIA_REG_8BIT: + case SMIAPP_REG_8BIT: data[2] = val; break; - case SMIA_REG_16BIT: + case SMIAPP_REG_16BIT: data[2] = val 8; data[3] = val; break; - case SMIA_REG_32BIT: + case SMIAPP_REG_32BIT: data[2] = val 24; data[3] = val 16; data[4] = val 8; diff --git a/drivers/media/i2c/smiapp/smiapp-regs.h b/drivers/media/i2c/smiapp/smiapp-regs.h index e07b30c..934130b 100644 --- a/drivers/media/i2c/smiapp/smiapp-regs.h +++ b/drivers/media/i2c/smiapp/smiapp-regs.h @@ -29,11 +29,11 @@ #include linux/types.h /* Use upper 8 bits of the type field for flags */ -#define SMIA_REG_FLAG_FLOAT(1 24) +#define
[PATCH v2 15/21] smiapp: Remove validation of op_pix_clk_div
op_pix_clk_div is directly assigned and not calculated. There's no need to verify it. Signed-off-by: Sakari Ailus sakari.ai...@linux.intel.com --- drivers/media/i2c/smiapp-pll.c | 5 - 1 file changed, 5 deletions(-) diff --git a/drivers/media/i2c/smiapp-pll.c b/drivers/media/i2c/smiapp-pll.c index bed44c0..6bde587 100644 --- a/drivers/media/i2c/smiapp-pll.c +++ b/drivers/media/i2c/smiapp-pll.c @@ -355,11 +355,6 @@ static int __smiapp_pll_calculate(struct device *dev, op_sys_clk_div); if (!rval) rval = bounds_check( - dev, pll-op_pix_clk_div, - limits-op.min_pix_clk_div, limits-op.max_pix_clk_div, - op_pix_clk_div); - if (!rval) - rval = bounds_check( dev, pll-op_sys_clk_freq_hz, limits-op.min_sys_clk_freq_hz, limits-op.max_sys_clk_freq_hz, -- 1.8.3.2 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 10/21] smiapp-pll: Correct clock debug prints
The PLL flags were not used correctly. Signed-off-by: Sakari Ailus sakari.ai...@linux.intel.com --- drivers/media/i2c/smiapp-pll.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/media/i2c/smiapp-pll.c b/drivers/media/i2c/smiapp-pll.c index 2335529..ab5d9a3 100644 --- a/drivers/media/i2c/smiapp-pll.c +++ b/drivers/media/i2c/smiapp-pll.c @@ -67,7 +67,7 @@ static void print_pll(struct device *dev, struct smiapp_pll *pll) { dev_dbg(dev, pre_pll_clk_div\t%d\n, pll-pre_pll_clk_div); dev_dbg(dev, pll_multiplier \t%d\n, pll-pll_multiplier); - if (pll-flags != SMIAPP_PLL_FLAG_NO_OP_CLOCKS) { + if (!(pll-flags SMIAPP_PLL_FLAG_NO_OP_CLOCKS)) { dev_dbg(dev, op_sys_clk_div \t%d\n, pll-op_sys_clk_div); dev_dbg(dev, op_pix_clk_div \t%d\n, pll-op_pix_clk_div); } @@ -77,7 +77,7 @@ static void print_pll(struct device *dev, struct smiapp_pll *pll) dev_dbg(dev, ext_clk_freq_hz \t%d\n, pll-ext_clk_freq_hz); dev_dbg(dev, pll_ip_clk_freq_hz \t%d\n, pll-pll_ip_clk_freq_hz); dev_dbg(dev, pll_op_clk_freq_hz \t%d\n, pll-pll_op_clk_freq_hz); - if (pll-flags SMIAPP_PLL_FLAG_NO_OP_CLOCKS) { + if (!(pll-flags SMIAPP_PLL_FLAG_NO_OP_CLOCKS)) { dev_dbg(dev, op_sys_clk_freq_hz \t%d\n, pll-op_sys_clk_freq_hz); dev_dbg(dev, op_pix_clk_freq_hz \t%d\n, -- 1.8.3.2 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[GIT PULL FOR v3.16] Various fixes
Hi Mauro, Various fixes, most notable are the vb2 fixes and davinci improvements. Regards, Hans The following changes since commit a83b93a7480441a47856dc9104bea970e84cda87: [media] em28xx-dvb: fix PCTV 461e tuner I2C binding (2014-03-31 08:02:16 -0300) are available in the git repository at: git://linuxtv.org/hverkuil/media_tree.git for-v3.16a for you to fetch changes up to a035f293b08859957c38086df3eb7fcf3a6b2272: v4l2-pci-skeleton.c: fix alternate field handling. (2014-04-14 10:54:28 +0200) Daniel Glöckner (1): bttv: Add support for PCI-8604PW Hans Verkuil (19): v4l2-subdev.h: fix sparse error with v4l2_subdev_notify videobuf2-core: fix sparse errors. v4l2-common.h: remove __user annotation in struct v4l2_edid v4l2-ioctl.c: fix sparse __user-related warnings v4l2-dv-timings.h: add CEA-861-F 4K timings v4l2-dv-timings.c: add the new 4K timings to the list. vb2: stop_streaming should return void vb2: fix handling of data_offset and v4l2_plane.reserved[] vb2: if bytesused is 0, then fill with output buffer length vb2: use correct prefix vb2: move __qbuf_mmap before __qbuf_userptr vb2: set timestamp when using write() vb2: reject output buffers with V4L2_FIELD_ALTERNATE vb2: simplify a confusing condition. vb2: add vb2_fileio_is_active and check it more often vb2: allow read/write as long as the format is single planar vb2: start messages with a lower-case for consistency. DocBook media: update bytesused field description v4l2-pci-skeleton.c: fix alternate field handling. Ismael Luceno (1): gspca_gl860: Clean up idxdata structs Lad, Prabhakar (4): media: davinci: vpbe: use v4l2_fh for priority handling media: davinci: vpfe: use v4l2_fh for priority handling staging: media: davinci: vpfe: use v4l2_fh for priority handling staging: media: davinci: vpfe: release buffers in case start_streaming call back fails Martin Bugge (2): adv7842: update RGB quantization range on HDMI/DVI-D mode irq. adv7842: Disable access to EDID DDC lines before chip power up. Mike Sampson (1): next-20140324 drivers/staging/media/sn9c102/sn9c102_hv7131r.c fix style warnings flagged by checkpatch.pl. ile...@telecom-paristech.fr (1): staging: omap24xx: fix coding style Documentation/DocBook/media/v4l/io.xml | 13 +- Documentation/video4linux/v4l2-pci-skeleton.c| 42 -- drivers/media/i2c/adv7842.c | 10 +- drivers/media/pci/bt8xx/bttv-cards.c | 110 ++ drivers/media/pci/bt8xx/bttv.h | 1 + drivers/media/pci/sta2x11/sta2x11_vip.c | 3 +- drivers/media/platform/blackfin/bfin_capture.c | 6 +- drivers/media/platform/coda.c| 4 +- drivers/media/platform/davinci/vpbe_display.c| 45 ++ drivers/media/platform/davinci/vpfe_capture.c| 13 +- drivers/media/platform/davinci/vpif_capture.c| 6 +- drivers/media/platform/davinci/vpif_display.c| 6 +- drivers/media/platform/exynos-gsc/gsc-m2m.c | 4 +- drivers/media/platform/exynos4-is/fimc-capture.c | 6 +- drivers/media/platform/exynos4-is/fimc-lite.c| 6 +- drivers/media/platform/exynos4-is/fimc-m2m.c | 3 +- drivers/media/platform/marvell-ccic/mcam-core.c | 7 +- drivers/media/platform/mem2mem_testdev.c | 5 +- drivers/media/platform/s3c-camif/camif-capture.c | 4 +- drivers/media/platform/s5p-jpeg/jpeg-core.c | 4 +- drivers/media/platform/s5p-mfc/s5p_mfc_dec.c | 3 +- drivers/media/platform/s5p-mfc/s5p_mfc_enc.c | 3 +- drivers/media/platform/s5p-tv/mixer_video.c | 3 +- drivers/media/platform/soc_camera/atmel-isi.c| 6 +- drivers/media/platform/soc_camera/mx2_camera.c | 4 +- drivers/media/platform/soc_camera/mx3_camera.c | 4 +- drivers/media/platform/soc_camera/rcar_vin.c | 4 +- drivers/media/platform/soc_camera/sh_mobile_ceu_camera.c | 4 +- drivers/media/platform/vivi.c| 3 +- drivers/media/platform/vsp1/vsp1_video.c | 4 +- drivers/media/usb/em28xx/em28xx-v4l.h| 2 +- drivers/media/usb/em28xx/em28xx-video.c | 8 +- drivers/media/usb/gspca/gl860/gl860-mi2020.c | 464 drivers/media/usb/pwc/pwc-if.c | 7 +- drivers/media/usb/s2255/s2255drv.c | 5 +- drivers/media/usb/stk1160/stk1160-v4l.c | 4 +- drivers/media/usb/usbtv/usbtv-video.c| 9
Re: stk1160 / ehci-pci 0000:00:0a.0: DMA-API: device driver maps memory fromstack [addr=ffff88003d0b56bf]
On Apr 13, Alan Stern wrote: On Sun, 13 Apr 2014, Sander Eikelenboom wrote: Hi, I'm hitting this warning on boot with a syntek usb video grabber, it's not clear to me if it's a driver issue of the stk1160 or a generic ehci issue. It is a bug in the stk1160 driver. Thanks for pointing this out, I'm on it. -- Ezequiel García, Free Electrons Embedded Linux, Kernel and Android Engineering http://free-electrons.com -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] uvcvideo: Work around buggy Logitech C920 firmware
On Mon, 14 Apr 2014, at 1:34, Laurent Pinchart wrote: On Thursday 13 March 2014 22:08:36 William Manley wrote: On 13/03/14 17:03, Laurent Pinchart wrote: On Thursday 13 March 2014 10:48:20 Will Manley wrote: On Thu, 13 Mar 2014, at 10:23, Laurent Pinchart wrote: On Wednesday 12 March 2014 18:08:31 William Manley wrote: The uvcvideo webcam driver exposes the v4l2 control Exposure (Absolute) which allows the user to control the exposure time of the webcam, essentially controlling the brightness of the received image. By default the webcam automatically adjusts the exposure time automatically but the if you set the control Exposure, Auto=Manual Mode the user can fix the exposure time. Unfortunately it seems that the Logitech C920 has a firmware bug where it will forget that it's in manual mode temporarily during initialisation. This means that the camera doesn't respect the exposure time that the user requested if they request it before starting to stream video. They end up with a video stream which is either too bright or too dark and must reset the controls after video starts streaming. I would like to get a better understanding of the problem first. As I don't have a C920, could you please perform two tests for me ? I would first like to know what the camera reports as its exposure time after starting the stream. If you get the exposure time at that point (by sending a GET_CUR request, bypassing the driver cache), do you get the value you had previously set (which, from your explanation, would be incorrect, as the exposure time has changed based on your findings), or a different value ? Does the camera change the exposure priority control autonomously as well, or just the exposure time ? It's a bit of a strange behaviour. I'd already tried littering the code with (uncached) GET_CUR requests. It seems that the value changes sometime during the call to usb_set_interface in uvc_init_video. I'll assume this means that the camera reports the updated exposure time in response to the GET_CUR request. That's right Does the value of other controls (such as the exposure priority control for instance) change as well ? No, I've not seen any of the other controls change. Strangely enough though calling uvc_ctrl_restore_values immediately after uvc_init_video has no effect. It must be put after the usb_submit_urb loop to fix the problem. Then, I would like to know whether the camera sends a control update event when you start the stream, or if it just changes the exposure time without notifying the driver. Wireshark tells me that it is sending a control update event, but it seems like uvcvideo ignores it. I had to add the flag UVC_CTRL_FLAG_AUTO_UPDATE to the uvc_control_info entry for Exposure (Auto) for the new value to be properly reported to userspace. Could you send me the USB trace ? I've uploaded 3 traces: one with no patch (kernel 3.12) one with my PATCH v2 applied and one with no patch but caching of gets disabled. You can get them here: http://williammanley.net/C920-no-patch.pcapng http://williammanley.net/C920-with-patch.pcapng http://williammanley.net/C920-no-caching.pcapng The process to generate them was: sudo dumpcap -i usbmon1 -w /tmp/C920-no-patch2.pcapng # Plug USB cable in here v4l2-ctl -d /dev/v4l/by-id/usb-046d_HD_Pro_Webcam_C920_C833389F-video-index1 -cexposure_auto=1,exposure_auto_priority=0,exposure_absolute=152,white_balan ce_temperature_auto=0 ./streamon The relevant output seems to be (here from C920-no-patch.pcapng): 10446 23.095874000 21:14:16.313257000 host - 32.0 USB 64 SET INTERFACE Request 10447 23.109379000 21:14:16.326762000 32.3 - host USBVIDEO 70 URB_INTERRUPT in 10448 23.109389000 21:14:16.326772000 host - 32.3 USB 64 URB_INTERRUPT in 10449 23.189448000 21:14:16.406831000 32.3 - host USBVIDEO 73 URB_INTERRUPT in 10450 23.189486000 21:14:16.406869000 host - 32.3 USB 64 URB_INTERRUPT in 10451 23.243314000 21:14:16.460697000 32.0 - host USB 64 SET INTERFACE Response Taking a closer look at packet 10449: Frame 10449: 73 bytes on wire (584 bits), 73 bytes captured (584 bits) on interface 0 Interface id: 0 Encapsulation type: USB packets with Linux header and padding (115) Arrival Time: Mar 13, 2014 21:14:16.406831000 GMT [Time shift for this packet: 0.0 seconds] Epoch Time: 1394745256.406831000 seconds [Time delta from previous captured frame: 0.080059000 seconds] [Time delta from previous displayed frame: 0.080059000 seconds] [Time since reference or first frame: 23.189448000 seconds] Frame Number: 10449 Frame Length:
[GIT PULL FOR v3.16] Various fixes
Hi Mauro, Various fixes, most notable are the vb2 fixes, saa7134 vb2 conversion and davinci improvements. This pull request supersedes https://patchwork.linuxtv.org/patch/23123/ and https://patchwork.linuxtv.org/patch/23596/. The saa7134 vb2 conversion patches had to be rebased on top of the vb2 fixes, so I combined it all into one pull request, ensuring a correct patch sequence. Regards, Hans The following changes since commit a83b93a7480441a47856dc9104bea970e84cda87: [media] em28xx-dvb: fix PCTV 461e tuner I2C binding (2014-03-31 08:02:16 -0300) are available in the git repository at: git://linuxtv.org/hverkuil/media_tree.git for-v3.16a for you to fetch changes up to 2500543b19714d169aae8930cf4f719e2343c819: saa7134: convert to vb2 (2014-04-14 12:35:11 +0200) Daniel Glöckner (1): bttv: Add support for PCI-8604PW Hans Verkuil (22): v4l2-subdev.h: fix sparse error with v4l2_subdev_notify videobuf2-core: fix sparse errors. v4l2-common.h: remove __user annotation in struct v4l2_edid v4l2-ioctl.c: fix sparse __user-related warnings v4l2-dv-timings.h: add CEA-861-F 4K timings v4l2-dv-timings.c: add the new 4K timings to the list. vb2: stop_streaming should return void vb2: fix handling of data_offset and v4l2_plane.reserved[] vb2: if bytesused is 0, then fill with output buffer length vb2: use correct prefix vb2: move __qbuf_mmap before __qbuf_userptr vb2: set timestamp when using write() vb2: reject output buffers with V4L2_FIELD_ALTERNATE vb2: simplify a confusing condition. vb2: add vb2_fileio_is_active and check it more often vb2: allow read/write as long as the format is single planar vb2: start messages with a lower-case for consistency. DocBook media: update bytesused field description v4l2-pci-skeleton.c: fix alternate field handling. vb2: add thread support vb2: Add videobuf2-dvb support saa7134: convert to vb2 Ismael Luceno (1): gspca_gl860: Clean up idxdata structs Lad, Prabhakar (4): media: davinci: vpbe: use v4l2_fh for priority handling media: davinci: vpfe: use v4l2_fh for priority handling staging: media: davinci: vpfe: use v4l2_fh for priority handling staging: media: davinci: vpfe: release buffers in case start_streaming call back fails Martin Bugge (2): adv7842: update RGB quantization range on HDMI/DVI-D mode irq. adv7842: Disable access to EDID DDC lines before chip power up. Mike Sampson (1): next-20140324 drivers/staging/media/sn9c102/sn9c102_hv7131r.c fix style warnings flagged by checkpatch.pl. ile...@telecom-paristech.fr (1): staging: omap24xx: fix coding style Documentation/DocBook/media/v4l/io.xml | 13 +- Documentation/video4linux/v4l2-pci-skeleton.c| 42 +++-- drivers/media/i2c/adv7842.c | 10 +- drivers/media/pci/bt8xx/bttv-cards.c | 110 +++ drivers/media/pci/bt8xx/bttv.h | 1 + drivers/media/pci/saa7134/Kconfig| 4 +- drivers/media/pci/saa7134/saa7134-alsa.c | 106 +-- drivers/media/pci/saa7134/saa7134-core.c | 107 ++- drivers/media/pci/saa7134/saa7134-dvb.c | 43 +++-- drivers/media/pci/saa7134/saa7134-empress.c | 179 +++--- drivers/media/pci/saa7134/saa7134-ts.c | 184 ++ drivers/media/pci/saa7134/saa7134-vbi.c | 170 - drivers/media/pci/saa7134/saa7134-video.c| 658 +++ drivers/media/pci/saa7134/saa7134.h | 106 +-- drivers/media/pci/sta2x11/sta2x11_vip.c | 3 +- drivers/media/platform/blackfin/bfin_capture.c | 6 +- drivers/media/platform/coda.c| 4 +- drivers/media/platform/davinci/vpbe_display.c| 45 + drivers/media/platform/davinci/vpfe_capture.c| 13 +- drivers/media/platform/davinci/vpif_capture.c| 6 +- drivers/media/platform/davinci/vpif_display.c| 6 +- drivers/media/platform/exynos-gsc/gsc-m2m.c | 4 +- drivers/media/platform/exynos4-is/fimc-capture.c | 6 +- drivers/media/platform/exynos4-is/fimc-lite.c| 6 +- drivers/media/platform/exynos4-is/fimc-m2m.c | 3 +- drivers/media/platform/marvell-ccic/mcam-core.c | 7 +- drivers/media/platform/mem2mem_testdev.c | 5 +- drivers/media/platform/s3c-camif/camif-capture.c | 4 +- drivers/media/platform/s5p-jpeg/jpeg-core.c | 4 +- drivers/media/platform/s5p-mfc/s5p_mfc_dec.c | 3 +-
Re: stk1160 / ehci-pci 0000:00:0a.0: DMA-API: device driver maps memory fromstack [addr=ffff88003d0b56bf]
On Apr 13, Sander Eikelenboom wrote: I'm hitting this warning on boot with a syntek usb video grabber, it's not clear to me if it's a driver issue of the stk1160 or a generic ehci issue. Can't reproduce the same warning easily here. Could you test the following patch? diff --git a/drivers/media/usb/stk1160/stk1160-core.c b/drivers/media/usb/stk1160/stk1160-core.c index 34a26e0..304fdb3 100644 --- a/drivers/media/usb/stk1160/stk1160-core.c +++ b/drivers/media/usb/stk1160/stk1160-core.c @@ -71,13 +71,14 @@ int stk1160_read_reg(struct stk1160 *dev, u16 reg, u8 *value) *value = 0; ret = usb_control_msg(dev-udev, pipe, 0x00, USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_DEVICE, - 0x00, reg, value, sizeof(u8), HZ); + 0x00, reg, dev-urb_buf, sizeof(u8), HZ); if (ret 0) { stk1160_err(read failed on reg 0x%x (%d)\n, reg, ret); return ret; } + *value = dev-urb_buf[0]; return 0; } -- Ezequiel García, Free Electrons Embedded Linux, Kernel and Android Engineering http://free-electrons.com -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [ANNOUNCE] media mini-summit on May, 2 in San Jose
Hi, From: linux-media-ow...@vger.kernel.org [mailto:linux-media- ow...@vger.kernel.org] On Behalf Of Mauro Carvalho Chehab Sent: Wednesday, March 19, 2014 8:02 PM As discussed on our IRC #v4l channels, most of the core developers will be in San Jose - CA - USA for the Embedded Linux Conference. There are several subjects that we've been discussing those days that require a face to face meeting. So, We'll be doing a media mini-summit on May, 2 (Friday) at Marriott San Jose. Eventually, we may also schedule some BoFs during the week, if needed. In order to properly organize the event, I need the name of the developers interested on joining us, plus the themes proposed for discussions. Sorry for joining so late. Me and Tomasz Figa will be attending the ELC and we would like to join the media mini summit. There were many interesting topics suggested. I would like to discuss in particular the Per-frame settings aka configuration stores suggested by Hans. Tomasz is working with the device tree so he could help us with Integration of the of-graph helpers and the component framework (drivers/base/component.c) in V4L2, suggested by Laurent. Best wishes, -- Kamil Debski Samsung RD Institute Poland As usual, we'll be using the media-works...@linuxtv.org ML for specific discussions about that, so the ones interested on participate are requested to subscribe it. Thanks! Mauro -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
OV7670: ENUM_FRAMESIZES seems buggy to me
Hi Jonathan, This loop in ov7670_enum_framesizes(): for (i = 0; i n_win_sizes; i++) { struct ov7670_win_size *win = info-devtype-win_sizes[index]; if (info-min_width win-width info-min_width) continue; if (info-min_height win-height info-min_height) continue; ... seems wrong to me. If any of the above if statements is true, it will stay true forever, until the loop terminates. If that's intended, you could at least use break immediately. If it's not - something else is wrong there. Maybe the win initialisation at the top of the loop should have i as an index? I.e. - struct ov7670_win_size *win = info-devtype-win_sizes[index]; + struct ov7670_win_size *win = info-devtype-win_sizes[i]; Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] V4L2: ov7670: fix a wrong index, potentially Oopsing the kernel from user-space
Commit 75e2bdad8901a0b599e01a96229be922eef1e488 ov7670: allow configuration of image size, clock speed, and I/O method uses a wrong index to iterate an array. Apart from being wrong, it also uses an unchecked value from user-space, which can cause access to unmapped memory in the kernel, triggered by a normal desktop user with rights to use V4L2 devices. Signed-off-by: Guennadi Liakhovetski g.liakhovet...@gmx.de --- Jonathan, I'd prefer to first post it to the lists to maybe have someone test it ;) Otherwise - I've got a couple more fixes for 3.15, which I hope to make ready and push in a couple of weeks... So, with your ack I can take this one too, or, if you prefer to push it earlier - would be good too. drivers/media/i2c/ov7670.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c index e8a1ce2..cdd7c1b 100644 --- a/drivers/media/i2c/ov7670.c +++ b/drivers/media/i2c/ov7670.c @@ -1109,7 +1109,7 @@ static int ov7670_enum_framesizes(struct v4l2_subdev *sd, * windows that fall outside that. */ for (i = 0; i n_win_sizes; i++) { - struct ov7670_win_size *win = info-devtype-win_sizes[index]; + struct ov7670_win_size *win = info-devtype-win_sizes[i]; if (info-min_width win-width info-min_width) continue; if (info-min_height win-height info-min_height) -- 1.9.1 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: OV7670: ENUM_FRAMESIZES seems buggy to me
On Mon, 14 Apr 2014 14:50:15 +0200 (CEST) Guennadi Liakhovetski g.liakhovet...@gmx.de wrote: If any of the above if statements is true, it will stay true forever, until the loop terminates. If that's intended, you could at least use break immediately. If it's not - something else is wrong there. Maybe the win initialisation at the top of the loop should have i as an index? I.e. - struct ov7670_win_size *win = info-devtype-win_sizes[index]; + struct ov7670_win_size *win = info-devtype-win_sizes[i]; Sigh. As far as I can tell, that bug was introduced by 75e2bdad8901a0b599e01a96229be922eef1e488 (ov7670: allow configuration of image size, clock speed, and I/O method) by Daniel Drake in 2.6.37. It's not only wrong, it could conceivably be a security issue - index is unchecked straight from user space. Say the word and I'll package up a patch. Otherwise please feel free to add my Acked-by to your own change, with a cc to stable@. Thanks for catching this, jon -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC v3 1/5] leds: Add sysfs and kernel internal API for flash LEDs
Hi Jacek, Thanks for the update! Some comments below. I'll try to reply to the rest during the coming days. On Fri, Apr 11, 2014 at 04:56:52PM +0200, Jacek Anaszewski wrote: Some LED devices support two operation modes - torch and flash. This patch provides support for flash LED devices in the LED subsystem by introducing new sysfs attributes and kernel internal interface. The attributes being introduced are: flash_brightness, flash_strobe, flash_timeout, max_flash_timeout, max_flash_brightness, flash_fault and optional external_strobe, indicator_brightness and max_indicator_btightness. All the flash related features are placed in a separate module. The modifications aim to be compatible with V4L2 framework requirements related to the flash devices management. The design assumes that V4L2 sub-device can take of the LED class device control and communicate with it through the kernel internal interface. When V4L2 Flash sub-device file is opened, the LED class device sysfs interface is made unavailable. Signed-off-by: Jacek Anaszewski j.anaszew...@samsung.com Acked-by: Kyungmin Park kyungmin.p...@samsung.com Cc: Bryan Wu coolo...@gmail.com Cc: Richard Purdie rpur...@rpsys.net --- drivers/leds/Kconfig|8 + drivers/leds/Makefile |1 + drivers/leds/led-class.c| 36 ++- drivers/leds/led-flash.c| 627 +++ drivers/leds/led-triggers.c | 16 +- drivers/leds/leds.h |6 + include/linux/leds.h| 50 +++- include/linux/leds_flash.h | 252 + 8 files changed, 982 insertions(+), 14 deletions(-) create mode 100644 drivers/leds/led-flash.c create mode 100644 include/linux/leds_flash.h diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig index 2062682..1e1c81f 100644 --- a/drivers/leds/Kconfig +++ b/drivers/leds/Kconfig @@ -19,6 +19,14 @@ config LEDS_CLASS This option enables the led sysfs class in /sys/class/leds. You'll need this to do anything useful with LEDs. If unsure, say N. +config LEDS_CLASS_FLASH + tristate Flash LEDs Support + depends on LEDS_CLASS + help + This option enables support for flash LED devices. Say Y if you + want to use flash specific features of a LED device, if they + are supported. + comment LED drivers config LEDS_88PM860X diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile index 3cd76db..8861b86 100644 --- a/drivers/leds/Makefile +++ b/drivers/leds/Makefile @@ -2,6 +2,7 @@ # LED Core obj-$(CONFIG_NEW_LEDS) += led-core.o obj-$(CONFIG_LEDS_CLASS) += led-class.o +obj-$(CONFIG_LEDS_CLASS_FLASH) += led-flash.o obj-$(CONFIG_LEDS_TRIGGERS) += led-triggers.o # LED Platform Drivers diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c index f37d63c..58f16c3 100644 --- a/drivers/leds/led-class.c +++ b/drivers/leds/led-class.c @@ -9,15 +9,16 @@ * published by the Free Software Foundation. */ -#include linux/module.h -#include linux/kernel.h +#include linux/ctype.h +#include linux/device.h +#include linux/err.h #include linux/init.h +#include linux/kernel.h #include linux/list.h +#include linux/module.h +#include linux/slab.h #include linux/spinlock.h -#include linux/device.h #include linux/timer.h -#include linux/err.h -#include linux/ctype.h #include linux/leds.h #include leds.h @@ -45,28 +46,38 @@ static ssize_t brightness_store(struct device *dev, { struct led_classdev *led_cdev = dev_get_drvdata(dev); unsigned long state; - ssize_t ret = -EINVAL; + ssize_t ret; + + mutex_lock(led_cdev-led_lock); + + if (led_sysfs_is_locked(led_cdev)) { + ret = -EBUSY; + goto unlock; + } ret = kstrtoul(buf, 10, state); if (ret) - return ret; + goto unlock; if (state == LED_OFF) led_trigger_remove(led_cdev); __led_set_brightness(led_cdev, state); + ret = size; - return size; +unlock: + mutex_unlock(led_cdev-led_lock); + return ret; } static DEVICE_ATTR_RW(brightness); -static ssize_t led_max_brightness_show(struct device *dev, +static ssize_t max_brightness_show(struct device *dev, struct device_attribute *attr, char *buf) { struct led_classdev *led_cdev = dev_get_drvdata(dev); return sprintf(buf, %u\n, led_cdev-max_brightness); } -static DEVICE_ATTR(max_brightness, 0444, led_max_brightness_show, NULL); +static DEVICE_ATTR_RO(max_brightness); #ifdef CONFIG_LEDS_TRIGGERS static DEVICE_ATTR(trigger, 0644, led_trigger_show, led_trigger_store); @@ -174,6 +185,8 @@ EXPORT_SYMBOL_GPL(led_classdev_suspend); void led_classdev_resume(struct led_classdev *led_cdev) { led_cdev-brightness_set(led_cdev, led_cdev-brightness); + if
[PATCH] cx23885: add support for Hauppauge ImpactVCB-e
This patch adds support for the Hauppauge ImpactVCB-e card to cx23885. Tested with Composite input and S-Video. While I do get audio it is very choppy. It is not clear whether that is a general cx23885 driver problem or specific to this board. If it is specific to the board, then I might have missed something. Steven (Toth, not Cookson ;-) ), do you have an idea what it might be? Regards, Hans Signed-off-by: Hans Verkuil hans.verk...@cisco.com --- drivers/media/pci/cx23885/cx23885-cards.c | 30 +- drivers/media/pci/cx23885/cx23885-video.c | 1 + drivers/media/pci/cx23885/cx23885.h | 1 + 3 files changed, 31 insertions(+), 1 deletion(-) diff --git a/drivers/media/pci/cx23885/cx23885-cards.c b/drivers/media/pci/cx23885/cx23885-cards.c index 79f20c8..49a3711 100644 --- a/drivers/media/pci/cx23885/cx23885-cards.c +++ b/drivers/media/pci/cx23885/cx23885-cards.c @@ -649,7 +649,25 @@ struct cx23885_board cx23885_boards[] = { CX25840_NONE1_CH3, .amux = CX25840_AUDIO6, } }, - } + }, + [CX23885_BOARD_HAUPPAUGE_IMPACTVCBE] = { + .name = Hauppauge ImpactVCB-e, + .porta = CX23885_ANALOG_VIDEO, + .input = {{ + .type = CX23885_VMUX_COMPOSITE1, + .vmux = CX25840_VIN7_CH3 | + CX25840_VIN4_CH2 | + CX25840_VIN6_CH1, + .amux = CX25840_AUDIO7, + }, { + .type = CX23885_VMUX_SVIDEO, + .vmux = CX25840_VIN7_CH3 | + CX25840_VIN4_CH2 | + CX25840_VIN8_CH1 | + CX25840_SVIDEO_ON, + .amux = CX25840_AUDIO7, + } }, + }, }; const unsigned int cx23885_bcount = ARRAY_SIZE(cx23885_boards); @@ -897,6 +915,10 @@ struct cx23885_subid cx23885_subids[] = { .subvendor = 0x1461, .subdevice = 0xd939, .card = CX23885_BOARD_AVERMEDIA_HC81R, + }, { + .subvendor = 0x0070, + .subdevice = 0x7133, + .card = CX23885_BOARD_HAUPPAUGE_IMPACTVCBE, }, }; const unsigned int cx23885_idcount = ARRAY_SIZE(cx23885_subids); @@ -977,6 +999,9 @@ static void hauppauge_eeprom(struct cx23885_dev *dev, u8 *eeprom_data) case 71009: /* WinTV-HVR1200 (PCIe, Retail, full height) * DVB-T and basic analog */ + case 71100: + /* WinTV-ImpactVCB-e (PCIe, Retail, half height) +* Basic analog */ case 71359: /* WinTV-HVR1200 (PCIe, OEM, half height) * DVB-T and basic analog */ @@ -1701,6 +1726,7 @@ void cx23885_card_setup(struct cx23885_dev *dev) case CX23885_BOARD_HAUPPAUGE_HVR1850: case CX23885_BOARD_HAUPPAUGE_HVR1290: case CX23885_BOARD_HAUPPAUGE_HVR4400: + case CX23885_BOARD_HAUPPAUGE_IMPACTVCBE: if (dev-i2c_bus[0].i2c_rc == 0) hauppauge_eeprom(dev, eeprom+0xc0); break; @@ -1807,6 +1833,7 @@ void cx23885_card_setup(struct cx23885_dev *dev) case CX23885_BOARD_HAUPPAUGE_HVR1200: case CX23885_BOARD_HAUPPAUGE_HVR1700: case CX23885_BOARD_HAUPPAUGE_HVR1400: + case CX23885_BOARD_HAUPPAUGE_IMPACTVCBE: case CX23885_BOARD_LEADTEK_WINFAST_PXDVR3200_H: case CX23885_BOARD_LEADTEK_WINFAST_PXPVR2200: case CX23885_BOARD_LEADTEK_WINFAST_PXDVR3200_H_XC4000: @@ -1835,6 +1862,7 @@ void cx23885_card_setup(struct cx23885_dev *dev) break; case CX23885_BOARD_HAUPPAUGE_HVR1250: case CX23885_BOARD_HAUPPAUGE_HVR1800: + case CX23885_BOARD_HAUPPAUGE_IMPACTVCBE: case CX23885_BOARD_HAUPPAUGE_HVR1800lp: case CX23885_BOARD_HAUPPAUGE_HVR1700: case CX23885_BOARD_LEADTEK_WINFAST_PXDVR3200_H: diff --git a/drivers/media/pci/cx23885/cx23885-video.c b/drivers/media/pci/cx23885/cx23885-video.c index 7891f34..976fe5d 100644 --- a/drivers/media/pci/cx23885/cx23885-video.c +++ b/drivers/media/pci/cx23885/cx23885-video.c @@ -507,6 +507,7 @@ static int cx23885_video_mux(struct cx23885_dev *dev, unsigned int input) if ((dev-board == CX23885_BOARD_HAUPPAUGE_HVR1800) || (dev-board == CX23885_BOARD_MPX885) || (dev-board == CX23885_BOARD_HAUPPAUGE_HVR1250) || + (dev-board == CX23885_BOARD_HAUPPAUGE_IMPACTVCBE) || (dev-board == CX23885_BOARD_HAUPPAUGE_HVR1255) || (dev-board == CX23885_BOARD_HAUPPAUGE_HVR1255_22111) || (dev-board == CX23885_BOARD_HAUPPAUGE_HVR1850) || diff --git a/drivers/media/pci/cx23885/cx23885.h
Re: [PATCH] cx23885: add support for Hauppauge ImpactVCB-e
Hi Hans, While I do get audio it is very choppy. It is not clear whether that is a general cx23885 driver problem or specific to this board. If it is specific to the board, then I might have missed something. Steven (Toth, not Cookson ;-) ), do you have an idea what it might be? Nothing comes to mind, I'm not aware of any audio issues with the other '885/7/8 boards. A board issue or driver regression would be my guess. - Steve -- Steven Toth - Kernel Labs http://www.kernellabs.com -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] media: davinci: vpbe: release buffers in case start_streaming call back fails
From: Lad, Prabhakar prabhakar.cse...@gmail.com this patch adds support to release the buffer by calling vb2_buffer_done(), with state marked as VB2_BUF_STATE_QUEUED if start_streaming() call back fails. Signed-off-by: Lad, Prabhakar prabhakar.cse...@gmail.com --- drivers/media/platform/davinci/vpbe_display.c | 11 ++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/drivers/media/platform/davinci/vpbe_display.c b/drivers/media/platform/davinci/vpbe_display.c index a9ad949..c8403ab 100644 --- a/drivers/media/platform/davinci/vpbe_display.c +++ b/drivers/media/platform/davinci/vpbe_display.c @@ -355,8 +355,17 @@ static int vpbe_start_streaming(struct vb2_queue *vq, unsigned int count) /* Set parameters in OSD and VENC */ ret = vpbe_set_osd_display_params(fh-disp_dev, layer); - if (ret 0) + if (ret 0) { + struct vpbe_disp_buffer *buf, *tmp; + + vb2_buffer_done(layer-cur_frm-vb, VB2_BUF_STATE_QUEUED); + list_for_each_entry_safe(buf, tmp, layer-dma_queue, list) { + list_del(buf-list); + vb2_buffer_done(buf-vb, VB2_BUF_STATE_QUEUED); + } + return ret; + } /* * if request format is yuv420 semiplanar, need to -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/4] drm: exynos: hdmi: add support for pixel clock limitation
Adds support for limitation of maximal pixel clock of HDMI signal. This feature is needed on boards that contains lines or bridges with frequency limitations. Signed-off-by: Tomasz Stanislawski t.stanisl...@samsung.com --- .../devicetree/bindings/video/exynos_hdmi.txt |4 drivers/gpu/drm/exynos/exynos_hdmi.c | 12 include/media/s5p_hdmi.h |1 + 3 files changed, 17 insertions(+) diff --git a/Documentation/devicetree/bindings/video/exynos_hdmi.txt b/Documentation/devicetree/bindings/video/exynos_hdmi.txt index f9187a2..8718f8d 100644 --- a/Documentation/devicetree/bindings/video/exynos_hdmi.txt +++ b/Documentation/devicetree/bindings/video/exynos_hdmi.txt @@ -28,6 +28,10 @@ Required properties: - ddc: phandle to the hdmi ddc node - phy: phandle to the hdmi phy node +Optional properties: +- max-pixel-clock: used to limit the maximal pixel clock if a board has lines, + connectors or bridges not capable of carring higher frequencies + Example: hdmi { diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c index 6fa63ea..ca313b3 100644 --- a/drivers/gpu/drm/exynos/exynos_hdmi.c +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c @@ -195,6 +195,7 @@ struct hdmi_context { struct hdmi_resources res; int hpd_gpio; + u32 max_pixel_clock; enum hdmi_type type; }; @@ -883,6 +884,9 @@ static int hdmi_mode_valid(struct drm_connector *connector, if (ret) return MODE_BAD; + if (mode-clock * 1000 hdata-max_pixel_clock) + return MODE_BAD; + ret = hdmi_find_phy_conf(hdata, mode-clock * 1000); if (ret 0) return MODE_BAD; @@ -2027,6 +2031,8 @@ static struct s5p_hdmi_platform_data *drm_hdmi_dt_parse_pdata return NULL; } + of_property_read_u32(np, max-pixel-clock, pd-max_pixel_clock); + return pd; } @@ -2063,6 +2069,11 @@ static int hdmi_probe(struct platform_device *pdev) if (!pdata) return -EINVAL; + if (!pdata-max_pixel_clock) { + DRM_INFO(max-pixel-clock is zero, using INF\n); + pdata-max_pixel_clock = ULONG_MAX; + } + hdata = devm_kzalloc(dev, sizeof(struct hdmi_context), GFP_KERNEL); if (!hdata) return -ENOMEM; @@ -2079,6 +2090,7 @@ static int hdmi_probe(struct platform_device *pdev) hdata-type = drv_data-type; hdata-hpd_gpio = pdata-hpd_gpio; + hdata-max_pixel_clock = pdata-max_pixel_clock; hdata-dev = dev; ret = hdmi_resources_init(hdata); diff --git a/include/media/s5p_hdmi.h b/include/media/s5p_hdmi.h index 181642b..7272d65 100644 --- a/include/media/s5p_hdmi.h +++ b/include/media/s5p_hdmi.h @@ -31,6 +31,7 @@ struct s5p_hdmi_platform_data { int mhl_bus; struct i2c_board_info *mhl_info; int hpd_gpio; + u32 max_pixel_clock; }; #endif /* S5P_HDMI_H */ -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/4] drm: exynos: hdmi: simplify extracting hpd-gpio from DT
This patch eliminates redundant checks while retrieving HPD gpio from DT during HDMI's probe(). Signed-off-by: Tomasz Stanislawski t.stanisl...@samsung.com --- drivers/gpu/drm/exynos/exynos_hdmi.c | 13 - 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c index 9a6d652..d2d6e2e 100644 --- a/drivers/gpu/drm/exynos/exynos_hdmi.c +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c @@ -2016,23 +2016,18 @@ static struct s5p_hdmi_platform_data *drm_hdmi_dt_parse_pdata { struct device_node *np = dev-of_node; struct s5p_hdmi_platform_data *pd; - u32 value; pd = devm_kzalloc(dev, sizeof(*pd), GFP_KERNEL); if (!pd) - goto err_data; + return NULL; - if (!of_find_property(np, hpd-gpio, value)) { + pd-hpd_gpio = of_get_named_gpio(np, hpd-gpio, 0); + if (gpio_is_valid(pd-hpd_gpio)) { DRM_ERROR(no hpd gpio property found\n); - goto err_data; + return NULL; } - pd-hpd_gpio = of_get_named_gpio(np, hpd-gpio, 0); - return pd; - -err_data: - return NULL; } static struct of_device_id hdmi_match_types[] = { -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/4] drm: exynos: update/fixes to HDMI driver
Hi everyone, This patchset adds 4 fixes/updates to EXYNOS DRM driver for HDMI subsystem. All comments are welcome. Regards, Tomasz Stanislawski Tomasz Stanislawski (4): drm: exynos: hdmi: simplify extracting hpd-gpio from DT drm: exynos: mixer: fix using usleep() in atomic context drm: exynos: add compatibles for HDMI and Mixer chips and exynos4210 SoC drm: exynos: hdmi: add support for pixel clock limitation .../devicetree/bindings/video/exynos_hdmi.txt |4 +++ drivers/gpu/drm/exynos/exynos_hdmi.c | 26 ++-- drivers/gpu/drm/exynos/exynos_mixer.c |5 +++- include/media/s5p_hdmi.h |1 + 4 files changed, 27 insertions(+), 9 deletions(-) -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/4] drm: exynos: add compatibles for HDMI and Mixer chips and exynos4210 SoC
This patch add proper compatibles for Mixer and HDMI chip available on exynos4210 SoCs. Signed-off-by: Tomasz Stanislawski t.stanisl...@samsung.com --- drivers/gpu/drm/exynos/exynos_hdmi.c |3 +++ drivers/gpu/drm/exynos/exynos_mixer.c |3 +++ 2 files changed, 6 insertions(+) diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c index d2d6e2e..6fa63ea 100644 --- a/drivers/gpu/drm/exynos/exynos_hdmi.c +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c @@ -2032,6 +2032,9 @@ static struct s5p_hdmi_platform_data *drm_hdmi_dt_parse_pdata static struct of_device_id hdmi_match_types[] = { { + .compatible = samsung,exynos4210-hdmi, + .data = (void *)HDMI_TYPE13, + }, { .compatible = samsung,exynos5-hdmi, .data = exynos5_hdmi_driver_data, }, { diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c index e3306c8..fd8a9a0 100644 --- a/drivers/gpu/drm/exynos/exynos_mixer.c +++ b/drivers/gpu/drm/exynos/exynos_mixer.c @@ -1187,6 +1187,9 @@ static struct platform_device_id mixer_driver_types[] = { static struct of_device_id mixer_match_types[] = { { + .compatible = samsung,exynos4210-mixer, + .data = exynos4210_mxr_drv_data, + }, { .compatible = samsung,exynos5-mixer, .data = exynos5250_mxr_drv_data, }, { -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/4] drm: exynos: mixer: fix using usleep() in atomic context
This patch fixes calling usleep_range() after taking reg_slock using spin_lock_irqsave(). The mdelay() is used instead. Waiting in atomic context is not the best idea in general. Hopefully, waiting occurs only when Video Processor fails to reset correctly. Signed-off-by: Tomasz Stanislawski t.stanisl...@samsung.com --- drivers/gpu/drm/exynos/exynos_mixer.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c index ce28881..e3306c8 100644 --- a/drivers/gpu/drm/exynos/exynos_mixer.c +++ b/drivers/gpu/drm/exynos/exynos_mixer.c @@ -615,7 +615,7 @@ static void vp_win_reset(struct mixer_context *ctx) /* waiting until VP_SRESET_PROCESSING is 0 */ if (~vp_reg_read(res, VP_SRESET) VP_SRESET_PROCESSING) break; - usleep_range(1, 12000); + mdelay(10); } WARN(tries == 0, failed to reset Video Processor\n); } -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/4] drm: exynos: hdmi: simplify extracting hpd-gpio from DT
On 04/14/2014 05:00 PM, Tomasz Stanislawski wrote: This patch eliminates redundant checks while retrieving HPD gpio from DT during HDMI's probe(). Signed-off-by: Tomasz Stanislawski t.stanisl...@samsung.com --- drivers/gpu/drm/exynos/exynos_hdmi.c | 13 - 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c index 9a6d652..d2d6e2e 100644 --- a/drivers/gpu/drm/exynos/exynos_hdmi.c +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c @@ -2016,23 +2016,18 @@ static struct s5p_hdmi_platform_data *drm_hdmi_dt_parse_pdata { struct device_node *np = dev-of_node; struct s5p_hdmi_platform_data *pd; - u32 value; pd = devm_kzalloc(dev, sizeof(*pd), GFP_KERNEL); if (!pd) - goto err_data; + return NULL; - if (!of_find_property(np, hpd-gpio, value)) { + pd-hpd_gpio = of_get_named_gpio(np, hpd-gpio, 0); + if (gpio_is_valid(pd-hpd_gpio)) { Sorry. Should be !gpio_is_valid(). DRM_ERROR(no hpd gpio property found\n); - goto err_data; + return NULL; } - pd-hpd_gpio = of_get_named_gpio(np, hpd-gpio, 0); - return pd; - -err_data: - return NULL; } static struct of_device_id hdmi_match_types[] = { -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: stk1160 / ehci-pci 0000:00:0a.0: DMA-API: device driver maps memory fromstack [addr=ffff88003d0b56bf]
On Apr 14, Ezequiel Garcia wrote: On Apr 13, Sander Eikelenboom wrote: I'm hitting this warning on boot with a syntek usb video grabber, it's not clear to me if it's a driver issue of the stk1160 or a generic ehci issue. Can't reproduce the same warning easily here. Could you test the following patch? Nevermind, just reproduced the warning. I'll be pushing a fix now. -- Ezequiel García, Free Electrons Embedded Linux, Kernel and Android Engineering http://free-electrons.com -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] media: stk1160: Avoid stack-allocated buffer for control URBs
Currently stk1160_read_reg() uses a stack-allocated char to get the read control value. This is wrong because usb_control_msg() requires a kmalloc-ed buffer, and a DMA-API warning is produced: WARNING: CPU: 0 PID: 1376 at lib/dma-debug.c:1153 check_for_stack+0xa0/0x100() ehci-pci :00:0a.0: DMA-API: device driver maps memory fromstack [addr=88003d0b56bf] This commit fixes such issue by using a 'usb_ctrl_read' field embedded in the device's struct to pass the value. In addition, we introduce a mutex to protect the value. While here, let's remove the urb_buf array which was meant for a similar purpose, but never really used. Reported-by: Sander Eikelenboom li...@eikelenboom.it Signed-off-by: Ezequiel Garcia ezequiel.gar...@free-electrons.com --- Sander, Hans: Does this cause any regressions, other than the DMA-API warning? In that case, we can consider this as suitable for -stable. drivers/media/usb/stk1160/stk1160-core.c | 8 +++- drivers/media/usb/stk1160/stk1160.h | 3 ++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/drivers/media/usb/stk1160/stk1160-core.c b/drivers/media/usb/stk1160/stk1160-core.c index 34a26e0..cce91e7 100644 --- a/drivers/media/usb/stk1160/stk1160-core.c +++ b/drivers/media/usb/stk1160/stk1160-core.c @@ -69,15 +69,20 @@ int stk1160_read_reg(struct stk1160 *dev, u16 reg, u8 *value) int pipe = usb_rcvctrlpipe(dev-udev, 0); *value = 0; + + mutex_lock(dev-urb_ctrl_lock); ret = usb_control_msg(dev-udev, pipe, 0x00, USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_DEVICE, - 0x00, reg, value, sizeof(u8), HZ); + 0x00, reg, dev-urb_ctrl_read, sizeof(u8), HZ); if (ret 0) { stk1160_err(read failed on reg 0x%x (%d)\n, reg, ret); + mutex_unlock(dev-urb_ctrl_lock); return ret; } + *value = dev-urb_ctrl_read; + mutex_unlock(dev-urb_ctrl_lock); return 0; } @@ -322,6 +327,7 @@ static int stk1160_probe(struct usb_interface *interface, * because we register the device node as the *last* thing. */ spin_lock_init(dev-buf_lock); + mutex_init(dev-urb_ctrl_lock); mutex_init(dev-v4l_lock); mutex_init(dev-vb_queue_lock); diff --git a/drivers/media/usb/stk1160/stk1160.h b/drivers/media/usb/stk1160/stk1160.h index 05b05b1..8886be4 100644 --- a/drivers/media/usb/stk1160/stk1160.h +++ b/drivers/media/usb/stk1160/stk1160.h @@ -143,7 +143,7 @@ struct stk1160 { int num_alt; struct stk1160_isoc_ctl isoc_ctl; - char urb_buf[255]; /* urb control msg buffer */ + char urb_ctrl_read; /* frame properties */ int width;/* current frame width */ @@ -159,6 +159,7 @@ struct stk1160 { struct i2c_adapter i2c_adap; struct i2c_client i2c_client; + struct mutex urb_ctrl_lock; /* protects urb_ctrl_read */ struct mutex v4l_lock; struct mutex vb_queue_lock; spinlock_t buf_lock; -- 1.9.1 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[GIT PULL] Fixes for v3.15
Hi Mauro, This includes a compilation error fix for s5c73m3 driver related to recently merged patch series moving the v4l2-of code and a memory allocation bug fix for the Exynos FIMC driver. The following changes since commit c9eaa447e77efe77b7fa4c953bd62de8297fd6c5: Linux 3.15-rc1 (2014-04-13 14:18:35 -0700) are available in the git repository at: ssh://linuxtv.org/git/snawrocki/samsung.git v3.15-fixes for you to fetch changes up to 45621f3b45650e98c704d598a4004355bc2cffb4: s5p-fimc: Fix YUV422P depth (2014-04-14 17:06:11 +0200) Nicolas Dufresne (1): s5p-fimc: Fix YUV422P depth Sylwester Nawrocki (1): s5c73m3: Add missing rename of v4l2_of_get_next_endpoint() function drivers/media/i2c/s5c73m3/s5c73m3-core.c |2 +- drivers/media/platform/exynos4-is/fimc-core.c |2 +- 2 files changed, 2 insertions(+), 2 deletions(-) -- Regards, Sylwester -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] media: stk1160: Avoid stack-allocated buffer for control URBs
Monday, April 14, 2014, 6:41:05 PM, you wrote: Currently stk1160_read_reg() uses a stack-allocated char to get the read control value. This is wrong because usb_control_msg() requires a kmalloc-ed buffer, and a DMA-API warning is produced: WARNING: CPU: 0 PID: 1376 at lib/dma-debug.c:1153 check_for_stack+0xa0/0x100() ehci-pci :00:0a.0: DMA-API: device driver maps memory fromstack [addr=88003d0b56bf] This commit fixes such issue by using a 'usb_ctrl_read' field embedded in the device's struct to pass the value. In addition, we introduce a mutex to protect the value. While here, let's remove the urb_buf array which was meant for a similar purpose, but never really used. Reported-by: Sander Eikelenboom li...@eikelenboom.it Signed-off-by: Ezequiel Garcia ezequiel.gar...@free-electrons.com --- Sander, Hans: Does this cause any regressions, other than the DMA-API warning? In that case, we can consider this as suitable for -stable. Thanks ! Will test and report back later. -- Sander drivers/media/usb/stk1160/stk1160-core.c | 8 +++- drivers/media/usb/stk1160/stk1160.h | 3 ++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/drivers/media/usb/stk1160/stk1160-core.c b/drivers/media/usb/stk1160/stk1160-core.c index 34a26e0..cce91e7 100644 --- a/drivers/media/usb/stk1160/stk1160-core.c +++ b/drivers/media/usb/stk1160/stk1160-core.c @@ -69,15 +69,20 @@ int stk1160_read_reg(struct stk1160 *dev, u16 reg, u8 *value) int pipe = usb_rcvctrlpipe(dev-udev, 0); *value = 0; + + mutex_lock(dev-urb_ctrl_lock); ret = usb_control_msg(dev-udev, pipe, 0x00, USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_DEVICE, - 0x00, reg, value, sizeof(u8), HZ); + 0x00, reg, dev-urb_ctrl_read, sizeof(u8), HZ); if (ret 0) { stk1160_err(read failed on reg 0x%x (%d)\n, reg, ret); + mutex_unlock(dev-urb_ctrl_lock); return ret; } + *value = dev-urb_ctrl_read; + mutex_unlock(dev-urb_ctrl_lock); return 0; } @@ -322,6 +327,7 @@ static int stk1160_probe(struct usb_interface *interface, * because we register the device node as the *last* thing. */ spin_lock_init(dev-buf_lock); + mutex_init(dev-urb_ctrl_lock); mutex_init(dev-v4l_lock); mutex_init(dev-vb_queue_lock); diff --git a/drivers/media/usb/stk1160/stk1160.h b/drivers/media/usb/stk1160/stk1160.h index 05b05b1..8886be4 100644 --- a/drivers/media/usb/stk1160/stk1160.h +++ b/drivers/media/usb/stk1160/stk1160.h @@ -143,7 +143,7 @@ struct stk1160 { int num_alt; struct stk1160_isoc_ctl isoc_ctl; - char urb_buf[255]; /* urb control msg buffer */ + char urb_ctrl_read; /* frame properties */ int width;/* current frame width */ @@ -159,6 +159,7 @@ struct stk1160 { struct i2c_adapter i2c_adap; struct i2c_client i2c_client; + struct mutex urb_ctrl_lock; /* protects urb_ctrl_read */ struct mutex v4l_lock; struct mutex vb_queue_lock; spinlock_t buf_lock; -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [v2] media: soc-camera: OF cameras
On Thu, Apr 10, 2014 at 2:18 PM, Guennadi Liakhovetski g.liakhovet...@gmx.de wrote: Hi Bryan, On Tue, 8 Apr 2014, Bryan Wu wrote: Thanks Josh, I think I will take you point and rework my patch again. But I need Guennadi's review firstly, Guennadi, could you please help to review it? Ok, let me double check the situation: 1. We've got this patch from you, aiming at adding OF probing support to soc-camra 2. We've got an alternative patch from Ben to do the same, his last reply to a comment to his patch was Thanks, I will look into this. 3. We've got Ben's patches for rcar-vin, that presumably work with his patch from (2) above 4. We've got Josh's patches to add OF / async probing to atmel-isi and ov2640, that are not known to work with either (1) or (2) above, so, they don't work at all, right? So, to summarise, there is a core patch from Ben, that he possibly wants to adjust, and that works with his rcar-vin OF, there is a patch from you that isn't known to work with any driver, and there are patches from Josh, that don't work, because there isn't a suitable patch available for them. I will have a look at your and Ben's soc-camera OF patches to compare them and compare them with my early code (hopefully this coming weekend), but so far it looks like only Ben's solution has a complete working stack. Am I missing something? My bad. I missed the conversation and patches from Ben Dooks and you guys. I have no problem for merging Ben's patch and I will align my Tegra Camera patch with that, probably posted later. Thanks, -Bryan -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] media: stk1160: Avoid stack-allocated buffer for control URBs
On Mon, 14 Apr 2014, Ezequiel Garcia wrote: Currently stk1160_read_reg() uses a stack-allocated char to get the read control value. This is wrong because usb_control_msg() requires a kmalloc-ed buffer, and a DMA-API warning is produced: WARNING: CPU: 0 PID: 1376 at lib/dma-debug.c:1153 check_for_stack+0xa0/0x100() ehci-pci :00:0a.0: DMA-API: device driver maps memory fromstack [addr=88003d0b56bf] This commit fixes such issue by using a 'usb_ctrl_read' field embedded in the device's struct to pass the value. In addition, we introduce a mutex to protect the value. This isn't right either. The buffer must be allocated in its own cache line; it must not be part of a larger structure. Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [v2] media: soc-camera: OF cameras
On 14/04/14 18:14, Bryan Wu wrote: On Thu, Apr 10, 2014 at 2:18 PM, Guennadi Liakhovetski g.liakhovet...@gmx.de wrote: Hi Bryan, On Tue, 8 Apr 2014, Bryan Wu wrote: Thanks Josh, I think I will take you point and rework my patch again. But I need Guennadi's review firstly, Guennadi, could you please help to review it? Ok, let me double check the situation: 1. We've got this patch from you, aiming at adding OF probing support to soc-camra 2. We've got an alternative patch from Ben to do the same, his last reply to a comment to his patch was Thanks, I will look into this. 3. We've got Ben's patches for rcar-vin, that presumably work with his patch from (2) above 4. We've got Josh's patches to add OF / async probing to atmel-isi and ov2640, that are not known to work with either (1) or (2) above, so, they don't work at all, right? So, to summarise, there is a core patch from Ben, that he possibly wants to adjust, and that works with his rcar-vin OF, there is a patch from you that isn't known to work with any driver, and there are patches from Josh, that don't work, because there isn't a suitable patch available for them. I will have a look at your and Ben's soc-camera OF patches to compare them and compare them with my early code (hopefully this coming weekend), but so far it looks like only Ben's solution has a complete working stack. Am I missing something? My bad. I missed the conversation and patches from Ben Dooks and you guys. I have no problem for merging Ben's patch and I will align my Tegra Camera patch with that, probably posted later. If possible, could you test the latest one? I've not had much time to actually use this and would welcome some feedback. -- Ben Dooks http://www.codethink.co.uk/ Senior Engineer Codethink - Providing Genius -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] media: stk1160: Avoid stack-allocated buffer for control URBs
On Apr 14, Alan Stern wrote: On Mon, 14 Apr 2014, Ezequiel Garcia wrote: Currently stk1160_read_reg() uses a stack-allocated char to get the read control value. This is wrong because usb_control_msg() requires a kmalloc-ed buffer, and a DMA-API warning is produced: WARNING: CPU: 0 PID: 1376 at lib/dma-debug.c:1153 check_for_stack+0xa0/0x100() ehci-pci :00:0a.0: DMA-API: device driver maps memory fromstack [addr=88003d0b56bf] This commit fixes such issue by using a 'usb_ctrl_read' field embedded in the device's struct to pass the value. In addition, we introduce a mutex to protect the value. This isn't right either. The buffer must be allocated in its own cache line; it must not be part of a larger structure. In that case, we can simply allocate 1 byte using kmalloc(). We won't be needing the mutex and it'll ensure proper cache alignment, right? -- Ezequiel García, Free Electrons Embedded Linux, Kernel and Android Engineering http://free-electrons.com -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] Helper to abstract vma handling in media layer
On Fri 11-04-14 08:58:59, Hans Verkuil wrote: On 04/11/2014 12:18 AM, Jan Kara wrote: On Thu 10-04-14 23:57:38, Jan Kara wrote: On Thu 10-04-14 14:22:20, Hans Verkuil wrote: On 04/10/14 14:15, Jan Kara wrote: On Thu 10-04-14 13:07:42, Hans Verkuil wrote: On 04/10/14 12:32, Jan Kara wrote: Hello, On Thu 10-04-14 12:02:50, Marek Szyprowski wrote: On 2014-03-17 20:49, Jan Kara wrote: The following patch series is my first stab at abstracting vma handling from the various media drivers. After this patch set drivers have to know much less details about vmas, their types, and locking. My motivation for the series is that I want to change get_user_pages() locking and I want to handle subtle locking details in as few places as possible. The core of the series is the new helper get_vaddr_pfns() which is given a virtual address and it fills in PFNs into provided array. If PFNs correspond to normal pages it also grabs references to these pages. The difference from get_user_pages() is that this function can also deal with pfnmap, mixed, and io mappings which is what the media drivers need. The patches are just compile tested (since I don't have any of the hardware I'm afraid I won't be able to do any more testing anyway) so please handle with care. I'm grateful for any comments. Thanks for posting this series! I will check if it works with our hardware soon. This is something I wanted to introduce some time ago to simplify buffer handling in dma-buf, but I had no time to start working. Thanks for having a look in the series. However I would like to go even further with integration of your pfn vector idea. This structure looks like a best solution for a compact representation of the memory buffer, which should be considered by the hardware as contiguous (either contiguous in physical memory or mapped contiguously into dma address space by the respective iommu). As you already noticed it is widely used by graphics and video drivers. I would also like to add support for pfn vector directly to the dma-mapping subsystem. This can be done quite easily (even with a fallback for architectures which don't provide method for it). I will try to prepare rfc soon. This will finally remove the need for hacks in media/v4l2-core/videobuf2-dma-contig.c That would be a worthwhile thing to do. When I was reading the code this seemed like something which could be done but I delibrately avoided doing more unification than necessary for my purposes as I don't have any hardware to test and don't know all the subtleties in the code... BTW, is there some way to test the drivers without the physical video HW? You can use the vivi driver (drivers/media/platform/vivi) for this. However, while the vivi driver can import dma buffers it cannot export them. If you want that, then you have to use this tree: http://git.linuxtv.org/cgit.cgi/hverkuil/media_tree.git/log/?h=vb2-part4 Thanks for the pointer that looks good. I've also found drivers/media/platform/mem2mem_testdev.c which seems to do even more testing of the area I made changes to. So now I have to find some userspace tool which can issue proper ioctls to setup and use the buffers and I can start testing what I wrote :) Get the v4l-utils.git repository (http://git.linuxtv.org/cgit.cgi/v4l-utils.git/). You want the v4l2-ctl tool. Don't use the version supplied by your distro, that's often too old. 'v4l2-ctl --help-streaming' gives the available options for doing streaming. So simple capturing from vivi is 'v4l2-ctl --stream-mmap' or '--stream-user'. You can't test dmabuf unless you switch to the vb2-part4 branch of my tree. Great, it seems to be doing something and it shows there's some bug in my code. Thanks a lot for help. OK, so after a small fix the basic functionality seems to be working. It doesn't seem there's a way to test multiplanar buffers with vivi, is there? For that you need to switch to the vb2-part4 branch as well. That has support for multiplanar. OK, I've merged that branch to my kernel but I failed to find the setting for vivi that would create multiplanar buffers and in fact I don't see multiplanar capabilities among the capabilities reported by the v4l2-ctl tool. Can you help me please? Honza -- Jan Kara j...@suse.cz SUSE Labs, CR -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 0/2] managed token devres interfaces
On 04/10/2014 08:39 AM, Mauro Carvalho Chehab wrote: Em Thu, 10 Apr 2014 12:46:53 +0100 One Thousand Gnomes gno...@lxorguk.ukuu.org.uk escreveu: For example, some devices provide standard USB Audio Class, handled by snd-usb-audio for the audio stream, while the video stream is handled via a separate driver, like some em28xx devices. Which is what mfd is designed to handle. There are even more complex devices that provide 3G modem, storage and digital TV, whose USB ID changes when either the 3G modem starts or when the digital TV firmware is loaded. But presumably you only have one driver at a time then ? The MFD device provides subdevices for all the functions. That is the whole underlying concept. I'll take a look on how it works, and how it locks resources on other drivers. What drivers outside drivers/mfd use shared resources that require locks controlled by mfd? One question that came up during the RFC review is: Can media drivers use mfd framework for sharing functions across drivers? I looked into the mfd framework and see if it helps the problem at hand hand. My conclusions as follows: 1. device presentation - media devices appear as a group of devices that can be clearly identified as independent devices. Each device implements a function which could be shared by one or more functions. - mfd devices appear as a single device. mfd framework helps present them as discrete platform devices. This model seems to be similar to a multi-function I/O card. This above is an important difference between these two device types. Media devices could benefit from using mfd framework in grouping the device to make it easier to treat these devices as a group. mfd framework could handle adding and removing devices. Something that could be looked into in detail to see if it makes sense to use mfd framework. 2. function sharing - each media device supports a function and one or more functions share another function. For example, tuner is shared by analog and digital TV functions. - mfd devices don't seem to work this way. Each function seems to be independent. Sharing is limited to the attach point. It works more like a bus, where sub-devices attach. I couldn't find any examples of a device/function being shared by other functions like in the case of media devices. 3. drivers - mfd/viperboard uses mfd to group gpio, i2c and does load viperboard i2c and viperboard gpio drivers. These drivers seem to have anything in common during run-time. i.e once mfd framework is used to carve the device up to appear as discrete devices, each device can function independently. - media drivers that drive one single media TV stick are a diversified group. In general, Analog and digital TV functions have to coordinate access to their shared functions. 4. Locking Both media and mfd drivers have mechanisms to lock/unlock hardware. mfd framework itself doesn't have any locking constructs. mfd drivers use mutex similar to the way media drivers use to protect access to hardware. This is a fine grain locking. However, media drivers need a large grain media function level locking mechanism, where a token devres will come in handy. I think this type of locking is needed even when media drivers take advantage of the mfd framework to group sub-devices. Another question to ask is: Why add token resources to drivers/base? Why not add at drivers/media? Some medial devices provide multiple almost-independent functions. USB and PCI core work is to allow multiple drivers to handle those almost-independent functions. For instance, snd-usb-audio driver will handle USB Audio Class devices, including some em28x devices, as it provides an independent UAC interface, while analog and digital TV are provided via another interface. What this means is a em28xx device could have snd-usb-audio driving the audio function. However, the audio stream is only available if the video stream is working. As a result, snd-usb-audio driver has to coordinate with the analog and digital functions em28xx_* drivers control. Hence, the need for a shared managed resource interfaces such as the proposed token devres interfaces at drivers/base level. This will allow a media device to be controlled by drivers that don't necessarily fall under drivers/media as in this above example. Webcams with microphone on the other hand have completely independent devices for audio and video. They don't need a shared devres type feature, as the USB core handles that properly. Based on the above, I think, the mfd framework will not help address the media driver problem at hand. However, I do think media drivers can benefit from using the mfd framework for streamlined handling of sub-devices. I would propose going forward with adding the token devres to solve the issue of sharing functions across drivers that control the functions. -- Shuah -- Shuah Khan Senior Linux Kernel Developer - Open
Re: [PATCH 3/4] drm: exynos: add compatibles for HDMI and Mixer chips and exynos4210 SoC
Hi Tomasz, On 04/15/2014 12:00 AM, Tomasz Stanislawski wrote: This patch add proper compatibles for Mixer and HDMI chip available on exynos4210 SoCs. Signed-off-by: Tomasz Stanislawski t.stanisl...@samsung.com --- drivers/gpu/drm/exynos/exynos_hdmi.c |3 +++ drivers/gpu/drm/exynos/exynos_mixer.c |3 +++ 2 files changed, 6 insertions(+) diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c index d2d6e2e..6fa63ea 100644 --- a/drivers/gpu/drm/exynos/exynos_hdmi.c +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c @@ -2032,6 +2032,9 @@ static struct s5p_hdmi_platform_data *drm_hdmi_dt_parse_pdata static struct of_device_id hdmi_match_types[] = { { + .compatible = samsung,exynos4210-hdmi, + .data = (void *)HDMI_TYPE13, It's consistent with others to use struct hdmi_driver_data like exynos5_hdmi_driver_data. + }, { .compatible = samsung,exynos5-hdmi, .data = exynos5_hdmi_driver_data, }, { diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c index e3306c8..fd8a9a0 100644 --- a/drivers/gpu/drm/exynos/exynos_mixer.c +++ b/drivers/gpu/drm/exynos/exynos_mixer.c @@ -1187,6 +1187,9 @@ static struct platform_device_id mixer_driver_types[] = { static struct of_device_id mixer_match_types[] = { { + .compatible = samsung,exynos4210-mixer, + .data = exynos4210_mxr_drv_data, + }, { .compatible = samsung,exynos5-mixer, .data = exynos5250_mxr_drv_data, }, { -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] uvc: update uvc_endpoint_max_bpi to handle USB_SPEED_WIRELESS devices
On Mon, 27 Jan 2014, Laurent Pinchart wrote: Hi Thomas, On Monday 27 January 2014 09:54:58 Thomas Pugliese wrote: On Mon, 27 Jan 2014, Laurent Pinchart wrote: On Friday 24 January 2014 15:17:28 Thomas Pugliese wrote: Isochronous endpoints on devices with speed == USB_SPEED_WIRELESS can have a max packet size ranging from 1-3584 bytes. Add a case to uvc_endpoint_max_bpi to handle USB_SPEED_WIRELESS. Otherwise endpoints for those devices will fall to the default case which masks off any values 2047. This causes uvc_init_video to underestimate the bandwidth available and fail to find a suitable alt setting for high bandwidth video streams. I'm not too familiar with wireless USB, but shouldn't the value be multiplied by bMaxBurst from the endpoint companion descriptor ? Superspeed devices provide the multiplied value in their endpoint companion descriptor's wBytesPerInterval field, but there's no such field for wireless devices. For wireless USB isochronous endpoints, the values in the endpoint descriptor are the logical interval and max packet size that the endpoint can support. They are provided for backwards compatibility for just this type of situation. You are correct that the actual endpoint characteristics are the bMaxBurst, wOverTheAirPacketSize, and bOverTheAirInterval values from the WUSB endpoint companion descriptor but only the host controller really needs to know about those details. In fact, the values from the endpoint companion descriptor might actually over-estimate the bandwidth available since the device can set bMaxBurst to a higher value than necessary to allow for retries. OK, I'll trust you on that :-) I've taken the patch in my tree and will send a pull request for v3.15. Out of curiosity, which device have you tested this with ? The device is a standard wired UVC webcam: Quanta CQEC2B (VID: 0x0408, PID: 0x9005). It is connected to an Alereon Wireless USB bridge dev kit which allows it to operate as a WUSB device. Thomas Signed-off-by: Thomas Pugliese thomas.pugli...@gmail.com --- drivers/media/usb/uvc/uvc_video.c | 3 +++ 1 file changed, 3 insertions(+) -- Regards, Laurent Pinchart So it turns out that this change (commit 79af67e77f86404e77e65ad954bf) breaks wireless USB devices that were designed to work with Windows because Windows also does not differentiate between Wireless USB devices and USB 2.0 high speed devices. This change should probably be reverted before it goes out in the 3.15 release. Devices that are strictly WUSB spec compliant will not work with some max packet sizes but they never did anyway. In order to support both compliant and non-compliant WUSB devices, uvc_endpoint_max_bpi should look at the endpoint companion descriptor but that descriptor is not readily available as it is for super speed devices so that patch will have to wait for another time. Thanks, Thomas -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
cron job: media_tree daily build: ERRORS
This message is generated daily by a cron job that builds media_tree for the kernels and architectures in the list below. Results of the daily build of media_tree: date: Tue Apr 15 04:00:26 CEST 2014 git branch: test git hash: 277a163c83d7ba93fba1e8980d29a9f8bfcfba6c gcc version:i686-linux-gcc (GCC) 4.8.2 sparse version: v0.5.0-11-g38d1124 host hardware: x86_64 host os:3.13-7.slh.1-amd64 linux-git-arm-at91: ERRORS linux-git-arm-davinci: ERRORS linux-git-arm-exynos: ERRORS linux-git-arm-mx: ERRORS linux-git-arm-omap: ERRORS linux-git-arm-omap1: ERRORS linux-git-arm-pxa: ERRORS linux-git-blackfin: ERRORS linux-git-i686: OK linux-git-m32r: OK linux-git-mips: ERRORS linux-git-powerpc64: OK linux-git-sh: ERRORS linux-git-x86_64: OK linux-2.6.31.14-i686: ERRORS linux-2.6.32.27-i686: ERRORS linux-2.6.33.7-i686: ERRORS linux-2.6.34.7-i686: ERRORS linux-2.6.35.9-i686: ERRORS linux-2.6.36.4-i686: ERRORS linux-2.6.37.6-i686: ERRORS linux-2.6.38.8-i686: ERRORS linux-2.6.39.4-i686: ERRORS linux-3.0.60-i686: ERRORS linux-3.1.10-i686: ERRORS linux-3.2.37-i686: ERRORS linux-3.3.8-i686: ERRORS linux-3.4.27-i686: ERRORS linux-3.5.7-i686: ERRORS linux-3.6.11-i686: ERRORS linux-3.7.4-i686: ERRORS linux-3.8-i686: ERRORS linux-3.9.2-i686: ERRORS linux-3.10.1-i686: ERRORS linux-3.11.1-i686: ERRORS linux-3.12-i686: ERRORS linux-3.13-i686: ERRORS linux-3.14-i686: ERRORS linux-2.6.31.14-x86_64: ERRORS linux-2.6.32.27-x86_64: ERRORS linux-2.6.33.7-x86_64: ERRORS linux-2.6.34.7-x86_64: ERRORS linux-2.6.35.9-x86_64: ERRORS linux-2.6.36.4-x86_64: ERRORS linux-2.6.37.6-x86_64: ERRORS linux-2.6.38.8-x86_64: ERRORS linux-2.6.39.4-x86_64: ERRORS linux-3.0.60-x86_64: ERRORS linux-3.1.10-x86_64: ERRORS linux-3.2.37-x86_64: ERRORS linux-3.3.8-x86_64: ERRORS linux-3.4.27-x86_64: ERRORS linux-3.5.7-x86_64: ERRORS linux-3.6.11-x86_64: ERRORS linux-3.7.4-x86_64: ERRORS linux-3.8-x86_64: ERRORS linux-3.9.2-x86_64: ERRORS linux-3.10.1-x86_64: ERRORS linux-3.11.1-x86_64: ERRORS linux-3.12-x86_64: ERRORS linux-3.13-x86_64: ERRORS linux-3.14-x86_64: ERRORS apps: OK spec-git: OK sparse version: v0.5.0-11-g38d1124 sparse: ERRORS Detailed results are available here: http://www.xs4all.nl/~hverkuil/logs/Tuesday.log Full logs are available here: http://www.xs4all.nl/~hverkuil/logs/Tuesday.tar.bz2 The Media Infrastructure API from this daily build is here: http://www.xs4all.nl/~hverkuil/spec/media.html -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From Dr.Paul Sawadogo
My name is Dr.Paul Sawadogo from Burkina Faso please I have a cancer patient in Coma who wants me to transfer her fortune worth 4.7 Million Dollars to you.Please get back to me for more details. -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] [media] s2255drv: fix memory leak s2255_probe()
smatch says: drivers/media/usb/s2255/s2255drv.c:2246 s2255_probe() warn: possible memory leak of 'dev' Signed-off-by: Daeseok Youn daeseok.y...@gmail.com --- drivers/media/usb/s2255/s2255drv.c |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/drivers/media/usb/s2255/s2255drv.c b/drivers/media/usb/s2255/s2255drv.c index 1d4ba2b..8aca3ef 100644 --- a/drivers/media/usb/s2255/s2255drv.c +++ b/drivers/media/usb/s2255/s2255drv.c @@ -2243,6 +2243,7 @@ static int s2255_probe(struct usb_interface *interface, dev-cmdbuf = kzalloc(S2255_CMDBUF_SIZE, GFP_KERNEL); if (dev-cmdbuf == NULL) { s2255_dev_err(interface-dev, out of memory\n); + kfree(dev); return -ENOMEM; } -- 1.7.4.4 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html