On 27/01/18 00:42, Douglas Fischer wrote: > Fixed si470x_start() disabling the interrupt signal, causing tune > operations to never complete. This does not affect USB radios > because they poll the registers instead of using the IRQ line. > > Signed-off-by: Douglas Fischer <fischerdouglasc@xxxxxxxxx> > --- > > diff -uprN linux.orig/drivers/media/radio/si470x/radio-si470x-common.c > linux/drivers/media/radio/si470x/radio-si470x-common.c --- > linux.orig/drivers/media/radio/si470x/radio-si470x-common.c > 2018-01-15 21:58:10.675620432 -0500 +++ > linux/drivers/media/radio/si470x/radio-si470x-common.c > 2018-01-16 16:54:23.699770645 -0500 @@ -377,8 +377,13 @@ int > si470x_start(struct si470x_device *r goto done; /* sysconfig 1 */ > - radio->registers[SYSCONFIG1] = > - (de << 11) & SYSCONFIG1_DE; /* DE*/ > + radio->registers[SYSCONFIG1] |= SYSCONFIG1_RDSIEN; > + radio->registers[SYSCONFIG1] |= SYSCONFIG1_STCIEN; > + radio->registers[SYSCONFIG1] |= SYSCONFIG1_RDS; Just do: radio->registers[SYSCONFIG1] |= SYSCONFIG1_RDSIEN | SYSCONFIG1_STCIEN | SYSCONFIG1_RDS; > + radio->registers[SYSCONFIG1] &= ~SYSCONFIG1_GPIO2; Why is this cleared? > + radio->registers[SYSCONFIG1] |= 0x1 << 2; What's this? It doesn't use a define, so either add one or add a comment. > + if (de) > + radio->registers[SYSCONFIG1] |= SYSCONFIG1_DE; > retval = si470x_set_register(radio, SYSCONFIG1); > if (retval < 0) > goto done; > Also, this is now set in si470x_start, so the same code can now be removed in si470x_fops_open for i2c. In general I would feel happier if you just add a 'bool is_i2c' argument to si470x_start and only change SYSCONFIG1 for the i2c case. Regards, Hans