This patch series is not ready yet. However, as there are some problems reported
when this driver got merged at linux-next, let me send it, in order to avoid
someone to address the same problems I already fixed.
There are still some things to be fixed there. For example, the bitfield
endiannes logic for MPEG-TS tables are wrong. My plan is to address those
along this week.
Also, Kaffeine is not properly detecting the audio streams. It will likely need
some additional patches for it to properly detect and work with SMPTE 302m
audio streams.
Mauro Carvalho Chehab (11):
media: vidtv: add modaliases for the bridge driver
media: vidtv: prefer using dev_foo() instead of pr_foo()
media: vidtv: fix 32-bit warnings
media: vidtv: fix frequency tuning logic
media: vidtv: add an initial channel frequency
media: vidtv: get rid of some endiannes nonsense
media: vidtv: remove a wrong endiannes check from s302m generator
media: vidtv: properly initialize the internal state struct
media: vidtv: add basic support for DVBv5 stats
media: vidtv: get rid of the work queue
media: vidtv: increment byte and block counters
.../media/test-drivers/vidtv/vidtv_bridge.c | 35 ++-
.../media/test-drivers/vidtv/vidtv_bridge.h | 3 +
.../media/test-drivers/vidtv/vidtv_channel.c | 23 +-
.../media/test-drivers/vidtv/vidtv_common.c | 4 +-
.../media/test-drivers/vidtv/vidtv_demod.c | 242 +++++++++---------
.../media/test-drivers/vidtv/vidtv_demod.h | 4 -
drivers/media/test-drivers/vidtv/vidtv_mux.c | 32 ++-
drivers/media/test-drivers/vidtv/vidtv_mux.h | 9 +-
drivers/media/test-drivers/vidtv/vidtv_pes.c | 12 +-
drivers/media/test-drivers/vidtv/vidtv_psi.c | 32 ---
.../media/test-drivers/vidtv/vidtv_s302m.c | 10 +-
.../media/test-drivers/vidtv/vidtv_tuner.c | 41 ++-
12 files changed, 221 insertions(+), 226 deletions(-)
--
2.26.2
The code there is already doing the right thing, as it uses
value & 0xff, value & 0xff00, which already ensures the
expected endiannes.
So, it doesn't make any sense to touch the order depending on
the CPU endiannes.
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---
drivers/media/test-drivers/vidtv/vidtv_s302m.c | 8 --------
1 file changed, 8 deletions(-)
diff --git a/drivers/media/test-drivers/vidtv/vidtv_s302m.c b/drivers/media/test-drivers/vidtv/vidtv_s302m.c
index 5ae2ede951eb..1bab2e231a55 100644
--- a/drivers/media/test-drivers/vidtv/vidtv_s302m.c
+++ b/drivers/media/test-drivers/vidtv/vidtv_s302m.c
@@ -352,14 +352,6 @@ static u32 vidtv_s302m_write_frame(struct vidtv_encoder *e,
f.data[3] = (sample & 0x0FF0) >> 4;
f.data[4] = (sample & 0xF000) >> 12;
- #ifdef __LITTLE_ENDIAN
- f.data[0] = reverse[f.data[0]];
- f.data[1] = reverse[f.data[1]];
- f.data[2] = reverse[f.data[2]];
- f.data[3] = reverse[f.data[3]];
- f.data[4] = reverse[f.data[4]];
- #endif
-
nbytes += vidtv_memcpy(e->encoder_buf,
e->encoder_buf_offset,
VIDTV_S302M_BUF_SZ,
--
2.26.2
Genmask is always highest order to low order. It doesn't make
any sense to make it depends on endiannes.
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---
drivers/media/test-drivers/vidtv/vidtv_psi.c | 32 --------------------
1 file changed, 32 deletions(-)
diff --git a/drivers/media/test-drivers/vidtv/vidtv_psi.c b/drivers/media/test-drivers/vidtv/vidtv_psi.c
index 761034d10d9d..b8b638244b1d 100644
--- a/drivers/media/test-drivers/vidtv/vidtv_psi.c
+++ b/drivers/media/test-drivers/vidtv/vidtv_psi.c
@@ -99,11 +99,7 @@ static inline u16 vidtv_psi_sdt_serv_get_desc_loop_len(struct vidtv_psi_table_sd
u16 mask;
u16 ret;
- #if defined(__BIG_ENDIAN)
- mask = GENMASK(0, 11);
- #else
mask = GENMASK(11, 0);
- #endif
ret = be16_to_cpu(s->bitfield) & mask;
return ret;
@@ -114,11 +110,7 @@ static inline u16 vidtv_psi_pmt_stream_get_desc_loop_len(struct vidtv_psi_table_
u16 mask;
u16 ret;
- #if defined(__BIG_ENDIAN)
- mask = GENMASK(0, 9);
- #else
mask = GENMASK(9, 0);
- #endif
ret = be16_to_cpu(s->bitfield2) & mask;
return ret;
@@ -129,11 +121,7 @@ static inline u16 vidtv_psi_pmt_get_desc_loop_len(struct vidtv_psi_table_pmt *p)
u16 mask;
u16 ret;
- #if defined(__BIG_ENDIAN)
- mask = GENMASK(0, 9);
- #else
mask = GENMASK(9, 0);
- #endif
ret = be16_to_cpu(p->bitfield2) & mask;
return ret;
@@ -144,11 +132,7 @@ static inline u16 vidtv_psi_get_sec_len(struct vidtv_psi_table_header *h)
u16 mask;
u16 ret;
- #if defined(__BIG_ENDIAN)
- mask = GENMASK(0, 11);
- #else
mask = GENMASK(11, 0);
- #endif
ret = be16_to_cpu(h->bitfield) & mask;
return ret;
@@ -159,11 +143,7 @@ inline u16 vidtv_psi_get_pat_program_pid(struct vidtv_psi_table_pat_program *p)
u16 mask;
u16 ret;
- #if defined(__BIG_ENDIAN)
- mask = GENMASK(0, 12);
- #else
mask = GENMASK(12, 0);
- #endif
ret = be16_to_cpu(p->bitfield) & mask;
return ret;
@@ -174,11 +154,7 @@ inline u16 vidtv_psi_pmt_stream_get_elem_pid(struct vidtv_psi_table_pmt_stream *
u16 mask;
u16 ret;
- #if defined(__BIG_ENDIAN)
- mask = GENMASK(0, 12);
- #else
mask = GENMASK(12, 0);
- #endif
ret = be16_to_cpu(s->bitfield) & mask;
return ret;
@@ -189,11 +165,7 @@ static inline void vidtv_psi_set_desc_loop_len(__be16 *bitfield, u16 new_len, u8
u16 mask;
__be16 new;
- #if defined(__BIG_ENDIAN)
- mask = GENMASK(desc_len_nbits, 15);
- #else
mask = GENMASK(15, desc_len_nbits);
- #endif
new = cpu_to_be16((be16_to_cpu(*bitfield) & mask) | new_len);
*bitfield = new;
@@ -205,11 +177,7 @@ static void vidtv_psi_set_sec_len(struct vidtv_psi_table_header *h, u16 new_len)
__be16 new;
u16 mask;
- #if defined(__BIG_ENDIAN)
- mask = GENMASK(13, 15);
- #else
mask = GENMASK(15, 13);
- #endif
new = cpu_to_be16((be16_to_cpu(h->bitfield) & mask) | new_len);
--
2.26.2
Right now, the config data passed from the bridge driver is
just ignored.
Also, let's initialize the delayed work at probing time.
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---
drivers/media/test-drivers/vidtv/vidtv_demod.c | 10 +++++-----
drivers/media/test-drivers/vidtv/vidtv_demod.h | 1 -
2 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/media/test-drivers/vidtv/vidtv_demod.c b/drivers/media/test-drivers/vidtv/vidtv_demod.c
index 3eb48b4a9a6b..6199a4e06ff9 100644
--- a/drivers/media/test-drivers/vidtv/vidtv_demod.c
+++ b/drivers/media/test-drivers/vidtv/vidtv_demod.c
@@ -268,10 +268,6 @@ static int vidtv_demod_init(struct dvb_frontend *fe)
struct vidtv_demod_state *state = fe->demodulator_priv;
u32 tuner_status = 0;
- if (state->cold_start)
- INIT_DELAYED_WORK(&state->poll_snr,
- &vidtv_demod_poll_snr_handler);
-
/*
* At resume, start the snr poll thread only if it was suspended with
* the thread running. Extra care should be taken here, as some tuner
@@ -288,7 +284,6 @@ static int vidtv_demod_init(struct dvb_frontend *fe)
state->poll_snr_thread_restart = false;
}
- state->cold_start = false;
return 0;
}
@@ -396,6 +391,7 @@ MODULE_DEVICE_TABLE(i2c, vidtv_demod_i2c_id_table);
static int vidtv_demod_i2c_probe(struct i2c_client *client,
const struct i2c_device_id *id)
{
+ struct vidtv_tuner_config *config = client->dev.platform_data;
struct vidtv_demod_state *state;
/* allocate memory for the internal state */
@@ -408,6 +404,10 @@ static int vidtv_demod_i2c_probe(struct i2c_client *client,
&vidtv_demod_ops,
sizeof(struct dvb_frontend_ops));
+ memcpy(&state->config, config, sizeof(state->config));
+
+ INIT_DELAYED_WORK(&state->poll_snr, &vidtv_demod_poll_snr_handler);
+
state->frontend.demodulator_priv = state;
i2c_set_clientdata(client, state);
diff --git a/drivers/media/test-drivers/vidtv/vidtv_demod.h b/drivers/media/test-drivers/vidtv/vidtv_demod.h
index dfb36c515e4d..7f52a537935b 100644
--- a/drivers/media/test-drivers/vidtv/vidtv_demod.h
+++ b/drivers/media/test-drivers/vidtv/vidtv_demod.h
@@ -66,7 +66,6 @@ struct vidtv_demod_state {
struct delayed_work poll_snr;
enum fe_status status;
u16 tuner_cnr;
- bool cold_start;
bool poll_snr_thread_running;
bool poll_snr_thread_restart;
};
--
2.26.2
Hi Mauro,
> Genmask is always highest order to low order. It doesn't make
> any sense to make it depends on endiannes.
>
I added these #ifdefs due to this:
https://lwn.net/Articles/741762/
i.e.
Fields to access are specified as GENMASK() values - an N-bit field
starting at bit #M is encoded as GENMASK(M + N - 1, N). Note that
bit numbers refer to endianness of the object we are working with -
e.g. GENMASK(11, 0) in __be16 refers to the second byte and the lower
4 bits of the first byte. In __le16 it would refer to the first byte
and the lower 4 bits of the second byte, etc.
I am not 100% sure, but maybe we actually need them?
- Daniel
Hi Daniel,
Em Mon, 14 Sep 2020 12:14:38 -0300
"Daniel W. S. Almeida" <[email protected]> escreveu:
> Hi Mauro,
>
> > Genmask is always highest order to low order. It doesn't make
> > any sense to make it depends on endiannes.
> >
>
> I added these #ifdefs due to this:
>
> https://lwn.net/Articles/741762/
>
> i.e.
>
> Fields to access are specified as GENMASK() values - an N-bit field
> starting at bit #M is encoded as GENMASK(M + N - 1, N). Note that
> bit numbers refer to endianness of the object we are working with -
> e.g. GENMASK(11, 0) in __be16 refers to the second byte and the lower
> 4 bits of the first byte. In __le16 it would refer to the first byte
> and the lower 4 bits of the second byte, etc.
>
> I am not 100% sure, but maybe we actually need them?
By looking at the changes you did with regards to bitfields,
it sounds that you didn't quite get how BE/LE works.
Basically, if the CPU needs to store a value (like 0x8001) on some
place, it will store two values: 0x80 and 0x01. Depending on the
endiannes, either 0x80 or 0x01 will be stored first. See:
https://en.wikipedia.org/wiki/Endianness
In any case, when you do something like:
mask = GENMASK(11, 0);
ret = be16_to_cpu(s->bitfield) & mask;
The be16_to_cpu() will ensure that the bits will be at the
position expected by the CPU endiannes. So, no need to check
for __BIG_ENDIAN or __LITTLE_ENDIAN when be*_to_cpu() macros
are used.
Please also notice that, when there's just one byte to be
stored (e. g. 8 bits), the endiannes won't matter, as the bits
will still be stored at the same way. that's why there's no
be8_to_cpu() or cpu_to_be8() macros.
Thanks,
Mauro