All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mauro Carvalho Chehab <mchehab-JsYNTwtnfakRB7SZvlqPiA@public.gmane.org>
To: Yasunari.Takiguchi-7U/KSKJipcs@public.gmane.org
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-media-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	tbird20d-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
	frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
	Masayuki Yamamoto
	<Masayuki.Yamamoto-7U/KSKJipcs@public.gmane.org>,
	Hideki Nozawa <Hideki.Nozawa-7U/KSKJipcs@public.gmane.org>,
	Kota Yonezawa <Kota.Yonezawa-7U/KSKJipcs@public.gmane.org>,
	Toshihiko Matsumoto
	<Toshihiko.Matsumoto-7U/KSKJipcs@public.gmane.org>,
	Satoshi Watanabe
	<Satoshi.C.Watanabe-7U/KSKJipcs@public.gmane.org>
Subject: Re: [PATCH v2 03/15] [media] cxd2880: Add common files for the driver
Date: Tue, 13 Jun 2017 09:13:01 -0300	[thread overview]
Message-ID: <20170613091301.3c8e63a9@vento.lan> (raw)
In-Reply-To: <20170414021701.17133-1-Yasunari.Takiguchi-7U/KSKJipcs@public.gmane.org>

Em Fri, 14 Apr 2017 11:17:01 +0900
<Yasunari.Takiguchi-7U/KSKJipcs@public.gmane.org> escreveu:

> From: Yasunari Takiguchi <Yasunari.Takiguchi-7U/KSKJipcs@public.gmane.org>
> 
> These are common files for the driver for the
> Sony CXD2880 DVB-T2/T tuner + demodulator.
> These contains helper functions for the driver.
> 
> Signed-off-by: Yasunari Takiguchi <Yasunari.Takiguchi-7U/KSKJipcs@public.gmane.org>
> Signed-off-by: Masayuki Yamamoto <Masayuki.Yamamoto-7U/KSKJipcs@public.gmane.org>
> Signed-off-by: Hideki Nozawa <Hideki.Nozawa-7U/KSKJipcs@public.gmane.org>
> Signed-off-by: Kota Yonezawa <Kota.Yonezawa-7U/KSKJipcs@public.gmane.org>
> Signed-off-by: Toshihiko Matsumoto <Toshihiko.Matsumoto-7U/KSKJipcs@public.gmane.org>
> Signed-off-by: Satoshi Watanabe <Satoshi.C.Watanabe-7U/KSKJipcs@public.gmane.org>
> ---
>  drivers/media/dvb-frontends/cxd2880/cxd2880.h      | 46 ++++++++++++
>  .../media/dvb-frontends/cxd2880/cxd2880_common.c   | 84 +++++++++++++++++++++
>  .../media/dvb-frontends/cxd2880/cxd2880_common.h   | 86 ++++++++++++++++++++++
>  drivers/media/dvb-frontends/cxd2880/cxd2880_io.c   | 68 +++++++++++++++++
>  drivers/media/dvb-frontends/cxd2880/cxd2880_io.h   | 62 ++++++++++++++++
>  .../media/dvb-frontends/cxd2880/cxd2880_stdlib.h   | 35 +++++++++
>  .../dvb-frontends/cxd2880/cxd2880_stopwatch_port.c | 71 ++++++++++++++++++
>  7 files changed, 452 insertions(+)
>  create mode 100644 drivers/media/dvb-frontends/cxd2880/cxd2880.h
>  create mode 100644 drivers/media/dvb-frontends/cxd2880/cxd2880_common.c
>  create mode 100644 drivers/media/dvb-frontends/cxd2880/cxd2880_common.h
>  create mode 100644 drivers/media/dvb-frontends/cxd2880/cxd2880_io.c
>  create mode 100644 drivers/media/dvb-frontends/cxd2880/cxd2880_io.h
>  create mode 100644 drivers/media/dvb-frontends/cxd2880/cxd2880_stdlib.h
>  create mode 100644 drivers/media/dvb-frontends/cxd2880/cxd2880_stopwatch_port.c
> 
> diff --git a/drivers/media/dvb-frontends/cxd2880/cxd2880.h b/drivers/media/dvb-frontends/cxd2880/cxd2880.h
> new file mode 100644
> index 000000000000..281f9a784eb5
> --- /dev/null
> +++ b/drivers/media/dvb-frontends/cxd2880/cxd2880.h
> @@ -0,0 +1,46 @@
> +/*
> + * cxd2880.h
> + * Sony CXD2880 DVB-T2/T tuner + demodulator driver public definitions
> + *
> + * Copyright (C) 2016, 2017 Sony Semiconductor Solutions Corporation
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the
> + * Free Software Foundation; version 2 of the License.
> + *
> + * THIS SOFTWARE IS PROVIDED ``AS IS'' AND ANY EXPRESS OR IMPLIED
> + * WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF
> + * MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN
> + * NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT,
> + * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
> + * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
> + * USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
> + * ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
> + * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> + *
> + * You should have received a copy of the GNU General Public License along
> + * with this program; if not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#ifndef CXD2880_H
> +#define CXD2880_H
> +
> +struct cxd2880_config {
> +	struct spi_device *spi;
> +	struct mutex *spi_mutex; /* For SPI access exclusive control */
> +};
> +
> +#if IS_REACHABLE(CONFIG_DVB_CXD2880)
> +extern struct dvb_frontend *cxd2880_attach(struct dvb_frontend *fe,
> +					struct cxd2880_config *cfg);
> +#else
> +static inline struct dvb_frontend *cxd2880_attach(struct dvb_frontend *fe,
> +					struct cxd2880_config *cfg)
> +{
> +	pr_warn("%s: driver disabled by Kconfig\n", __func__);
> +	return NULL;
> +}
> +#endif /* CONFIG_DVB_CXD2880 */
> +
> +#endif /* CXD2880_H */
> diff --git a/drivers/media/dvb-frontends/cxd2880/cxd2880_common.c b/drivers/media/dvb-frontends/cxd2880/cxd2880_common.c
> new file mode 100644
> index 000000000000..850f3a76b2c7
> --- /dev/null
> +++ b/drivers/media/dvb-frontends/cxd2880/cxd2880_common.c
> @@ -0,0 +1,84 @@
> +/*
> + * cxd2880_common.c
> + * Sony CXD2880 DVB-T2/T tuner + demodulator driver
> + * common functions
> + *
> + * Copyright (C) 2016, 2017 Sony Semiconductor Solutions Corporation
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the
> + * Free Software Foundation; version 2 of the License.
> + *
> + * THIS SOFTWARE IS PROVIDED ``AS IS'' AND ANY EXPRESS OR IMPLIED
> + * WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF
> + * MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN
> + * NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT,
> + * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
> + * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
> + * USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
> + * ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
> + * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> + *
> + * You should have received a copy of the GNU General Public License along
> + * with this program; if not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include "cxd2880_common.h"
> +
> +#define MASKUPPER(n) (((n) == 0) ? 0 : (0xFFFFFFFFU << (32 - (n))))
> +#define MASKLOWER(n) (((n) == 0) ? 0 : (0xFFFFFFFFU >> (32 - (n))))

Please prefer using the already existing bit macros at:
	include/linux/bitfield.h
	include/linux/bitops.h

> +
> +int cxd2880_convert2s_complement(u32 value, u32 bitlen)
> +{
> +	if ((bitlen == 0) || (bitlen >= 32))
> +		return (int)value;
> +
> +	if (value & (u32)(1 << (bitlen - 1)))
> +		return (int)(MASKUPPER(32 - bitlen) | value);
> +	else
> +		return (int)(MASKLOWER(bitlen) & value);
> +}
> +
> +u32 cxd2880_bit_split_from_byte_array(u8 *array, u32 start_bit, u32 bit_num)
> +{
> +	u32 value = 0;
> +	u8 *array_read;
> +	u8 bit_read;
> +	u32 len_remain;
> +
> +	if (!array)
> +		return 0;
> +	if ((bit_num == 0) || (bit_num > 32))
> +		return 0;
> +
> +	array_read = array + (start_bit / 8);
> +	bit_read = (u8)(start_bit % 8);
> +	len_remain = bit_num;
> +
> +	if (bit_read != 0) {
> +		if (((int)len_remain) <= 8 - bit_read) {
> +			value = (*array_read) >> ((8 - bit_read) - len_remain);
> +			len_remain = 0;
> +		} else {
> +			value = *array_read++;
> +			len_remain -= 8 - bit_read;
> +		}
> +	}
> +
> +	while (len_remain > 0) {
> +		if (len_remain < 8) {
> +			value <<= len_remain;
> +			value |= (*array_read++ >> (8 - len_remain));
> +			len_remain = 0;
> +		} else {
> +			value <<= 8;
> +			value |= (u32)(*array_read++);
> +			len_remain -= 8;
> +		}
> +	}
> +
> +	value &= MASKLOWER(bit_num);
> +
> +	return value;
> +}

The Kernel has already optimized macros to handle with bit arrays.

I suspect that you could get what you're doing above with this
macro (defined at include/linux/bitops.h):
	for_each_set_bit()

The Kernel macros usually take advantage of some machine instructions
to find bits, if available at the specific architecture the driver is
compiled.

> diff --git a/drivers/media/dvb-frontends/cxd2880/cxd2880_common.h b/drivers/media/dvb-frontends/cxd2880/cxd2880_common.h
> new file mode 100644
> index 000000000000..b1ecb44bca10
> --- /dev/null
> +++ b/drivers/media/dvb-frontends/cxd2880/cxd2880_common.h
> @@ -0,0 +1,86 @@
> +/*
> + * cxd2880_common.h
> + * Sony CXD2880 DVB-T2/T tuner + demodulator driver common definitions
> + *
> + * Copyright (C) 2016, 2017 Sony Semiconductor Solutions Corporation
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the
> + * Free Software Foundation; version 2 of the License.
> + *
> + * THIS SOFTWARE IS PROVIDED ``AS IS'' AND ANY EXPRESS OR IMPLIED
> + * WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF
> + * MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN
> + * NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT,
> + * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
> + * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
> + * USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
> + * ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
> + * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> + *
> + * You should have received a copy of the GNU General Public License along
> + * with this program; if not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#ifndef CXD2880_COMMON_H
> +#define CXD2880_COMMON_H
> +
> +#include <linux/types.h>
> +
> +#ifndef NULL
> +#ifdef __cplusplus
> +#define NULL 0
> +#else
> +#define NULL ((void *)0)
> +#endif
> +#endif

Please remove the above. NULL is already defined, and we don't compile the
Kernel on c++.

> +
> +#include <linux/delay.h>

> +#define CXD2880_SLEEP(n) msleep(n)
> +#ifndef CXD2880_SLEEP_IN_MON
> +#define CXD2880_SLEEP_IN_MON(n, obj) CXD2880_SLEEP(n)
> +#endif
> +
> +#define CXD2880_ARG_UNUSED(arg) ((void)(arg))

Please get rid of the above too, as the above just obfuscates the
code.

> +
> +enum cxd2880_ret {
> +	CXD2880_RESULT_OK,
> +	CXD2880_RESULT_ERROR_ARG,
> +	CXD2880_RESULT_ERROR_IO,
> +	CXD2880_RESULT_ERROR_SW_STATE,
> +	CXD2880_RESULT_ERROR_HW_STATE,
> +	CXD2880_RESULT_ERROR_TIMEOUT,
> +	CXD2880_RESULT_ERROR_UNLOCK,
> +	CXD2880_RESULT_ERROR_RANGE,
> +	CXD2880_RESULT_ERROR_NOSUPPORT,
> +	CXD2880_RESULT_ERROR_CANCEL,
> +	CXD2880_RESULT_ERROR_OTHER,
> +	CXD2880_RESULT_ERROR_OVERFLOW,
> +	CXD2880_RESULT_OK_CONFIRM
> +};

Please don't define your own return codes. Use the Posix-based error
codes, e. g.:

0 for OK
-EINVAL
-EOVERFLOW
...


> +
> +int cxd2880_convert2s_complement(u32 value, u32 bitlen);
> +
> +u32 cxd2880_bit_split_from_byte_array(u8 *array, u32 start_bit, u32 bit_num);
> +
> +struct cxd2880_atomic {
> +	int counter;
> +};
> +
> +#define cxd2880_atomic_set(a, i) ((a)->counter = i)
> +#define cxd2880_atomic_read(a) ((a)->counter)

No. Use the macros at include/linux/atomic.h for atomic operations.

> +
> +struct cxd2880_stopwatch {
> +	u32 start_time;
> +};
> +
> +enum cxd2880_ret cxd2880_stopwatch_start(struct cxd2880_stopwatch *stopwatch);
> +
> +enum cxd2880_ret cxd2880_stopwatch_sleep(struct cxd2880_stopwatch *stopwatch,
> +					 u32 ms);
> +
> +enum cxd2880_ret cxd2880_stopwatch_elapsed(struct cxd2880_stopwatch *stopwatch,
> +					   u32 *elapsed);
> +
> +#endif
> diff --git a/drivers/media/dvb-frontends/cxd2880/cxd2880_io.c b/drivers/media/dvb-frontends/cxd2880/cxd2880_io.c
> new file mode 100644
> index 000000000000..f0f82055a953
> --- /dev/null
> +++ b/drivers/media/dvb-frontends/cxd2880/cxd2880_io.c
> @@ -0,0 +1,68 @@
> +/*
> + * cxd2880_io.c
> + * Sony CXD2880 DVB-T2/T tuner + demodulator driver
> + * register I/O interface functions
> + *
> + * Copyright (C) 2016, 2017 Sony Semiconductor Solutions Corporation
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the
> + * Free Software Foundation; version 2 of the License.
> + *
> + * THIS SOFTWARE IS PROVIDED ``AS IS'' AND ANY EXPRESS OR IMPLIED
> + * WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF
> + * MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN
> + * NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT,
> + * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
> + * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
> + * USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
> + * ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
> + * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> + *
> + * You should have received a copy of the GNU General Public License along
> + * with this program; if not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include "cxd2880_io.h"
> +
> +enum cxd2880_ret cxd2880_io_common_write_one_reg(struct cxd2880_io *io,
> +						 enum cxd2880_io_tgt tgt,
> +						 u8 sub_address, u8 data)
> +{
> +	enum cxd2880_ret ret = CXD2880_RESULT_OK;
> +
> +	if (!io)
> +		return CXD2880_RESULT_ERROR_ARG;
> +
> +	ret = io->write_regs(io, tgt, sub_address, &data, 1);
> +
> +	return ret;
> +}

Here (and below): use the standard linux error codes.

> +
> +enum cxd2880_ret cxd2880_io_set_reg_bits(struct cxd2880_io *io,
> +					 enum cxd2880_io_tgt tgt,
> +					 u8 sub_address, u8 data, u8 mask)
> +{
> +	enum cxd2880_ret ret = CXD2880_RESULT_OK;
> +
> +	if (!io)
> +		return CXD2880_RESULT_ERROR_ARG;
> +
> +	if (mask == 0x00)
> +		return CXD2880_RESULT_OK;
> +
> +	if (mask != 0xFF) {

nitpick: we usually prefer hex numbers in low cases. In any case, please
be coherent along the driver.

> +		u8 rdata = 0x00;
> +
> +		ret = io->read_regs(io, tgt, sub_address, &rdata, 1);
> +		if (ret != CXD2880_RESULT_OK)
> +			return ret;
> +
> +		data = (u8)((data & mask) | (rdata & (mask ^ 0xFF)));
> +	}
> +
> +	ret = io->write_reg(io, tgt, sub_address, data);
> +
> +	return ret;
> +}
> diff --git a/drivers/media/dvb-frontends/cxd2880/cxd2880_io.h b/drivers/media/dvb-frontends/cxd2880/cxd2880_io.h
> new file mode 100644
> index 000000000000..4d6db13cf910
> --- /dev/null
> +++ b/drivers/media/dvb-frontends/cxd2880/cxd2880_io.h
> @@ -0,0 +1,62 @@
> +/*
> + * cxd2880_io.h
> + * Sony CXD2880 DVB-T2/T tuner + demodulator driver
> + * register I/O interface definitions
> + *
> + * Copyright (C) 2016, 2017 Sony Semiconductor Solutions Corporation
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the
> + * Free Software Foundation; version 2 of the License.
> + *
> + * THIS SOFTWARE IS PROVIDED ``AS IS'' AND ANY EXPRESS OR IMPLIED
> + * WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF
> + * MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN
> + * NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT,
> + * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
> + * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
> + * USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
> + * ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
> + * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> + *
> + * You should have received a copy of the GNU General Public License along
> + * with this program; if not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#ifndef CXD2880_IO_H
> +#define CXD2880_IO_H
> +
> +#include "cxd2880_common.h"
> +
> +enum cxd2880_io_tgt {
> +	CXD2880_IO_TGT_SYS,
> +	CXD2880_IO_TGT_DMD
> +};
> +
> +struct cxd2880_io {
> +	enum cxd2880_ret (*read_regs)(struct cxd2880_io *io,
> +				       enum cxd2880_io_tgt tgt, u8 sub_address,
> +				       u8 *data, u32 size);
> +	enum cxd2880_ret (*write_regs)(struct cxd2880_io *io,
> +					enum cxd2880_io_tgt tgt, u8 sub_address,
> +					const u8 *data, u32 size);
> +	enum cxd2880_ret (*write_reg)(struct cxd2880_io *io,
> +				       enum cxd2880_io_tgt tgt, u8 sub_address,
> +				       u8 data);
> +	void *if_object;
> +	u8 i2c_address_sys;
> +	u8 i2c_address_demod;
> +	u8 slave_select;
> +	void *user;
> +};
> +
> +enum cxd2880_ret cxd2880_io_common_write_one_reg(struct cxd2880_io *io,
> +						 enum cxd2880_io_tgt tgt,
> +						 u8 sub_address, u8 data);
> +
> +enum cxd2880_ret cxd2880_io_set_reg_bits(struct cxd2880_io *io,
> +					 enum cxd2880_io_tgt tgt,
> +					 u8 sub_address, u8 data, u8 mask);
> +
> +#endif
> diff --git a/drivers/media/dvb-frontends/cxd2880/cxd2880_stdlib.h b/drivers/media/dvb-frontends/cxd2880/cxd2880_stdlib.h
> new file mode 100644
> index 000000000000..b9ca1b9df110
> --- /dev/null
> +++ b/drivers/media/dvb-frontends/cxd2880/cxd2880_stdlib.h
> @@ -0,0 +1,35 @@
> +/*
> + * cxd2880_stdlib.h
> + * Sony CXD2880 DVB-T2/T tuner + demodulator driver
> + * standard lib function aliases
> + *
> + * Copyright (C) 2016, 2017 Sony Semiconductor Solutions Corporation
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the
> + * Free Software Foundation; version 2 of the License.
> + *
> + * THIS SOFTWARE IS PROVIDED ``AS IS'' AND ANY EXPRESS OR IMPLIED
> + * WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF
> + * MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN
> + * NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT,
> + * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
> + * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
> + * USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
> + * ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
> + * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> + *
> + * You should have received a copy of the GNU General Public License along
> + * with this program; if not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#ifndef CXD2880_STDLIB_H
> +#define CXD2880_STDLIB_H
> +
> +#include <linux/string.h>
> +
> +#define cxd2880_memcpy  memcpy
> +#define cxd2880_memset  memset

No. Just use memcpy()/memset() and get rid of this header.

> +
> +#endif
> diff --git a/drivers/media/dvb-frontends/cxd2880/cxd2880_stopwatch_port.c b/drivers/media/dvb-frontends/cxd2880/cxd2880_stopwatch_port.c
> new file mode 100644
> index 000000000000..14ad6aa6c4c0
> --- /dev/null
> +++ b/drivers/media/dvb-frontends/cxd2880/cxd2880_stopwatch_port.c
> @@ -0,0 +1,71 @@
> +/*
> + * cxd2880_stopwatch_port.c
> + * Sony CXD2880 DVB-T2/T tuner + demodulator driver
> + * time measurement functions
> + *
> + * Copyright (C) 2016, 2017 Sony Semiconductor Solutions Corporation
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the
> + * Free Software Foundation; version 2 of the License.
> + *
> + * THIS SOFTWARE IS PROVIDED ``AS IS'' AND ANY EXPRESS OR IMPLIED
> + * WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF
> + * MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN
> + * NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT,
> + * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
> + * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
> + * USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
> + * ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
> + * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> + *
> + * You should have received a copy of the GNU General Public License along
> + * with this program; if not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include "cxd2880_common.h"
> +
> +#include <linux/ktime.h>
> +#include <linux/time.h>
> +#include <linux/timekeeping.h>
> +
> +static u32 get_time_count(void)
> +{
> +	struct timespec tp;
> +
> +	getnstimeofday(&tp);

As said before, don't use it for timeout checks.

> +
> +	return (u32)((tp.tv_sec * 1000) + (tp.tv_nsec / 1000000));
> +}
> +
> +enum cxd2880_ret cxd2880_stopwatch_start(struct cxd2880_stopwatch *stopwatch)
> +{
> +	if (!stopwatch)
> +		return CXD2880_RESULT_ERROR_ARG;
> +
> +	stopwatch->start_time = get_time_count();
> +
> +	return CXD2880_RESULT_OK;
> +}
> +
> +enum cxd2880_ret cxd2880_stopwatch_sleep(struct cxd2880_stopwatch *stopwatch,
> +					 u32 ms)
> +{
> +	if (!stopwatch)
> +		return CXD2880_RESULT_ERROR_ARG;
> +	CXD2880_ARG_UNUSED(*stopwatch);
> +	CXD2880_SLEEP(ms);
> +
> +	return CXD2880_RESULT_OK;
> +}
> +
> +enum cxd2880_ret cxd2880_stopwatch_elapsed(struct cxd2880_stopwatch *stopwatch,
> +					   u32 *elapsed)
> +{
> +	if (!stopwatch || !elapsed)
> +		return CXD2880_RESULT_ERROR_ARG;
> +	*elapsed = get_time_count() - stopwatch->start_time;
> +
> +	return CXD2880_RESULT_OK;
> +}

You should reimplement the above using jiffies.

Thanks,
Mauro
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

WARNING: multiple messages have this Message-ID (diff)
From: Mauro Carvalho Chehab <mchehab@s-opensource.com>
To: <Yasunari.Takiguchi@sony.com>
Cc: <linux-kernel@vger.kernel.org>, <devicetree@vger.kernel.org>,
	<linux-media@vger.kernel.org>, <tbird20d@gmail.com>,
	<frowand.list@gmail.com>,
	Masayuki Yamamoto <Masayuki.Yamamoto@sony.com>,
	Hideki Nozawa <Hideki.Nozawa@sony.com>,
	"Kota Yonezawa" <Kota.Yonezawa@sony.com>,
	Toshihiko Matsumoto <Toshihiko.Matsumoto@sony.com>,
	Satoshi Watanabe <Satoshi.C.Watanabe@sony.com>
Subject: Re: [PATCH v2 03/15] [media] cxd2880: Add common files for the driver
Date: Tue, 13 Jun 2017 09:13:01 -0300	[thread overview]
Message-ID: <20170613091301.3c8e63a9@vento.lan> (raw)
In-Reply-To: <20170414021701.17133-1-Yasunari.Takiguchi@sony.com>

Em Fri, 14 Apr 2017 11:17:01 +0900
<Yasunari.Takiguchi@sony.com> escreveu:

> From: Yasunari Takiguchi <Yasunari.Takiguchi@sony.com>
> 
> These are common files for the driver for the
> Sony CXD2880 DVB-T2/T tuner + demodulator.
> These contains helper functions for the driver.
> 
> Signed-off-by: Yasunari Takiguchi <Yasunari.Takiguchi@sony.com>
> Signed-off-by: Masayuki Yamamoto <Masayuki.Yamamoto@sony.com>
> Signed-off-by: Hideki Nozawa <Hideki.Nozawa@sony.com>
> Signed-off-by: Kota Yonezawa <Kota.Yonezawa@sony.com>
> Signed-off-by: Toshihiko Matsumoto <Toshihiko.Matsumoto@sony.com>
> Signed-off-by: Satoshi Watanabe <Satoshi.C.Watanabe@sony.com>
> ---
>  drivers/media/dvb-frontends/cxd2880/cxd2880.h      | 46 ++++++++++++
>  .../media/dvb-frontends/cxd2880/cxd2880_common.c   | 84 +++++++++++++++++++++
>  .../media/dvb-frontends/cxd2880/cxd2880_common.h   | 86 ++++++++++++++++++++++
>  drivers/media/dvb-frontends/cxd2880/cxd2880_io.c   | 68 +++++++++++++++++
>  drivers/media/dvb-frontends/cxd2880/cxd2880_io.h   | 62 ++++++++++++++++
>  .../media/dvb-frontends/cxd2880/cxd2880_stdlib.h   | 35 +++++++++
>  .../dvb-frontends/cxd2880/cxd2880_stopwatch_port.c | 71 ++++++++++++++++++
>  7 files changed, 452 insertions(+)
>  create mode 100644 drivers/media/dvb-frontends/cxd2880/cxd2880.h
>  create mode 100644 drivers/media/dvb-frontends/cxd2880/cxd2880_common.c
>  create mode 100644 drivers/media/dvb-frontends/cxd2880/cxd2880_common.h
>  create mode 100644 drivers/media/dvb-frontends/cxd2880/cxd2880_io.c
>  create mode 100644 drivers/media/dvb-frontends/cxd2880/cxd2880_io.h
>  create mode 100644 drivers/media/dvb-frontends/cxd2880/cxd2880_stdlib.h
>  create mode 100644 drivers/media/dvb-frontends/cxd2880/cxd2880_stopwatch_port.c
> 
> diff --git a/drivers/media/dvb-frontends/cxd2880/cxd2880.h b/drivers/media/dvb-frontends/cxd2880/cxd2880.h
> new file mode 100644
> index 000000000000..281f9a784eb5
> --- /dev/null
> +++ b/drivers/media/dvb-frontends/cxd2880/cxd2880.h
> @@ -0,0 +1,46 @@
> +/*
> + * cxd2880.h
> + * Sony CXD2880 DVB-T2/T tuner + demodulator driver public definitions
> + *
> + * Copyright (C) 2016, 2017 Sony Semiconductor Solutions Corporation
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the
> + * Free Software Foundation; version 2 of the License.
> + *
> + * THIS SOFTWARE IS PROVIDED ``AS IS'' AND ANY EXPRESS OR IMPLIED
> + * WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF
> + * MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN
> + * NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT,
> + * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
> + * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
> + * USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
> + * ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
> + * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> + *
> + * You should have received a copy of the GNU General Public License along
> + * with this program; if not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#ifndef CXD2880_H
> +#define CXD2880_H
> +
> +struct cxd2880_config {
> +	struct spi_device *spi;
> +	struct mutex *spi_mutex; /* For SPI access exclusive control */
> +};
> +
> +#if IS_REACHABLE(CONFIG_DVB_CXD2880)
> +extern struct dvb_frontend *cxd2880_attach(struct dvb_frontend *fe,
> +					struct cxd2880_config *cfg);
> +#else
> +static inline struct dvb_frontend *cxd2880_attach(struct dvb_frontend *fe,
> +					struct cxd2880_config *cfg)
> +{
> +	pr_warn("%s: driver disabled by Kconfig\n", __func__);
> +	return NULL;
> +}
> +#endif /* CONFIG_DVB_CXD2880 */
> +
> +#endif /* CXD2880_H */
> diff --git a/drivers/media/dvb-frontends/cxd2880/cxd2880_common.c b/drivers/media/dvb-frontends/cxd2880/cxd2880_common.c
> new file mode 100644
> index 000000000000..850f3a76b2c7
> --- /dev/null
> +++ b/drivers/media/dvb-frontends/cxd2880/cxd2880_common.c
> @@ -0,0 +1,84 @@
> +/*
> + * cxd2880_common.c
> + * Sony CXD2880 DVB-T2/T tuner + demodulator driver
> + * common functions
> + *
> + * Copyright (C) 2016, 2017 Sony Semiconductor Solutions Corporation
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the
> + * Free Software Foundation; version 2 of the License.
> + *
> + * THIS SOFTWARE IS PROVIDED ``AS IS'' AND ANY EXPRESS OR IMPLIED
> + * WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF
> + * MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN
> + * NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT,
> + * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
> + * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
> + * USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
> + * ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
> + * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> + *
> + * You should have received a copy of the GNU General Public License along
> + * with this program; if not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include "cxd2880_common.h"
> +
> +#define MASKUPPER(n) (((n) == 0) ? 0 : (0xFFFFFFFFU << (32 - (n))))
> +#define MASKLOWER(n) (((n) == 0) ? 0 : (0xFFFFFFFFU >> (32 - (n))))

Please prefer using the already existing bit macros at:
	include/linux/bitfield.h
	include/linux/bitops.h

> +
> +int cxd2880_convert2s_complement(u32 value, u32 bitlen)
> +{
> +	if ((bitlen == 0) || (bitlen >= 32))
> +		return (int)value;
> +
> +	if (value & (u32)(1 << (bitlen - 1)))
> +		return (int)(MASKUPPER(32 - bitlen) | value);
> +	else
> +		return (int)(MASKLOWER(bitlen) & value);
> +}
> +
> +u32 cxd2880_bit_split_from_byte_array(u8 *array, u32 start_bit, u32 bit_num)
> +{
> +	u32 value = 0;
> +	u8 *array_read;
> +	u8 bit_read;
> +	u32 len_remain;
> +
> +	if (!array)
> +		return 0;
> +	if ((bit_num == 0) || (bit_num > 32))
> +		return 0;
> +
> +	array_read = array + (start_bit / 8);
> +	bit_read = (u8)(start_bit % 8);
> +	len_remain = bit_num;
> +
> +	if (bit_read != 0) {
> +		if (((int)len_remain) <= 8 - bit_read) {
> +			value = (*array_read) >> ((8 - bit_read) - len_remain);
> +			len_remain = 0;
> +		} else {
> +			value = *array_read++;
> +			len_remain -= 8 - bit_read;
> +		}
> +	}
> +
> +	while (len_remain > 0) {
> +		if (len_remain < 8) {
> +			value <<= len_remain;
> +			value |= (*array_read++ >> (8 - len_remain));
> +			len_remain = 0;
> +		} else {
> +			value <<= 8;
> +			value |= (u32)(*array_read++);
> +			len_remain -= 8;
> +		}
> +	}
> +
> +	value &= MASKLOWER(bit_num);
> +
> +	return value;
> +}

The Kernel has already optimized macros to handle with bit arrays.

I suspect that you could get what you're doing above with this
macro (defined at include/linux/bitops.h):
	for_each_set_bit()

The Kernel macros usually take advantage of some machine instructions
to find bits, if available at the specific architecture the driver is
compiled.

> diff --git a/drivers/media/dvb-frontends/cxd2880/cxd2880_common.h b/drivers/media/dvb-frontends/cxd2880/cxd2880_common.h
> new file mode 100644
> index 000000000000..b1ecb44bca10
> --- /dev/null
> +++ b/drivers/media/dvb-frontends/cxd2880/cxd2880_common.h
> @@ -0,0 +1,86 @@
> +/*
> + * cxd2880_common.h
> + * Sony CXD2880 DVB-T2/T tuner + demodulator driver common definitions
> + *
> + * Copyright (C) 2016, 2017 Sony Semiconductor Solutions Corporation
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the
> + * Free Software Foundation; version 2 of the License.
> + *
> + * THIS SOFTWARE IS PROVIDED ``AS IS'' AND ANY EXPRESS OR IMPLIED
> + * WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF
> + * MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN
> + * NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT,
> + * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
> + * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
> + * USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
> + * ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
> + * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> + *
> + * You should have received a copy of the GNU General Public License along
> + * with this program; if not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#ifndef CXD2880_COMMON_H
> +#define CXD2880_COMMON_H
> +
> +#include <linux/types.h>
> +
> +#ifndef NULL
> +#ifdef __cplusplus
> +#define NULL 0
> +#else
> +#define NULL ((void *)0)
> +#endif
> +#endif

Please remove the above. NULL is already defined, and we don't compile the
Kernel on c++.

> +
> +#include <linux/delay.h>

> +#define CXD2880_SLEEP(n) msleep(n)
> +#ifndef CXD2880_SLEEP_IN_MON
> +#define CXD2880_SLEEP_IN_MON(n, obj) CXD2880_SLEEP(n)
> +#endif
> +
> +#define CXD2880_ARG_UNUSED(arg) ((void)(arg))

Please get rid of the above too, as the above just obfuscates the
code.

> +
> +enum cxd2880_ret {
> +	CXD2880_RESULT_OK,
> +	CXD2880_RESULT_ERROR_ARG,
> +	CXD2880_RESULT_ERROR_IO,
> +	CXD2880_RESULT_ERROR_SW_STATE,
> +	CXD2880_RESULT_ERROR_HW_STATE,
> +	CXD2880_RESULT_ERROR_TIMEOUT,
> +	CXD2880_RESULT_ERROR_UNLOCK,
> +	CXD2880_RESULT_ERROR_RANGE,
> +	CXD2880_RESULT_ERROR_NOSUPPORT,
> +	CXD2880_RESULT_ERROR_CANCEL,
> +	CXD2880_RESULT_ERROR_OTHER,
> +	CXD2880_RESULT_ERROR_OVERFLOW,
> +	CXD2880_RESULT_OK_CONFIRM
> +};

Please don't define your own return codes. Use the Posix-based error
codes, e. g.:

0 for OK
-EINVAL
-EOVERFLOW
...


> +
> +int cxd2880_convert2s_complement(u32 value, u32 bitlen);
> +
> +u32 cxd2880_bit_split_from_byte_array(u8 *array, u32 start_bit, u32 bit_num);
> +
> +struct cxd2880_atomic {
> +	int counter;
> +};
> +
> +#define cxd2880_atomic_set(a, i) ((a)->counter = i)
> +#define cxd2880_atomic_read(a) ((a)->counter)

No. Use the macros at include/linux/atomic.h for atomic operations.

> +
> +struct cxd2880_stopwatch {
> +	u32 start_time;
> +};
> +
> +enum cxd2880_ret cxd2880_stopwatch_start(struct cxd2880_stopwatch *stopwatch);
> +
> +enum cxd2880_ret cxd2880_stopwatch_sleep(struct cxd2880_stopwatch *stopwatch,
> +					 u32 ms);
> +
> +enum cxd2880_ret cxd2880_stopwatch_elapsed(struct cxd2880_stopwatch *stopwatch,
> +					   u32 *elapsed);
> +
> +#endif
> diff --git a/drivers/media/dvb-frontends/cxd2880/cxd2880_io.c b/drivers/media/dvb-frontends/cxd2880/cxd2880_io.c
> new file mode 100644
> index 000000000000..f0f82055a953
> --- /dev/null
> +++ b/drivers/media/dvb-frontends/cxd2880/cxd2880_io.c
> @@ -0,0 +1,68 @@
> +/*
> + * cxd2880_io.c
> + * Sony CXD2880 DVB-T2/T tuner + demodulator driver
> + * register I/O interface functions
> + *
> + * Copyright (C) 2016, 2017 Sony Semiconductor Solutions Corporation
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the
> + * Free Software Foundation; version 2 of the License.
> + *
> + * THIS SOFTWARE IS PROVIDED ``AS IS'' AND ANY EXPRESS OR IMPLIED
> + * WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF
> + * MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN
> + * NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT,
> + * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
> + * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
> + * USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
> + * ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
> + * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> + *
> + * You should have received a copy of the GNU General Public License along
> + * with this program; if not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include "cxd2880_io.h"
> +
> +enum cxd2880_ret cxd2880_io_common_write_one_reg(struct cxd2880_io *io,
> +						 enum cxd2880_io_tgt tgt,
> +						 u8 sub_address, u8 data)
> +{
> +	enum cxd2880_ret ret = CXD2880_RESULT_OK;
> +
> +	if (!io)
> +		return CXD2880_RESULT_ERROR_ARG;
> +
> +	ret = io->write_regs(io, tgt, sub_address, &data, 1);
> +
> +	return ret;
> +}

Here (and below): use the standard linux error codes.

> +
> +enum cxd2880_ret cxd2880_io_set_reg_bits(struct cxd2880_io *io,
> +					 enum cxd2880_io_tgt tgt,
> +					 u8 sub_address, u8 data, u8 mask)
> +{
> +	enum cxd2880_ret ret = CXD2880_RESULT_OK;
> +
> +	if (!io)
> +		return CXD2880_RESULT_ERROR_ARG;
> +
> +	if (mask == 0x00)
> +		return CXD2880_RESULT_OK;
> +
> +	if (mask != 0xFF) {

nitpick: we usually prefer hex numbers in low cases. In any case, please
be coherent along the driver.

> +		u8 rdata = 0x00;
> +
> +		ret = io->read_regs(io, tgt, sub_address, &rdata, 1);
> +		if (ret != CXD2880_RESULT_OK)
> +			return ret;
> +
> +		data = (u8)((data & mask) | (rdata & (mask ^ 0xFF)));
> +	}
> +
> +	ret = io->write_reg(io, tgt, sub_address, data);
> +
> +	return ret;
> +}
> diff --git a/drivers/media/dvb-frontends/cxd2880/cxd2880_io.h b/drivers/media/dvb-frontends/cxd2880/cxd2880_io.h
> new file mode 100644
> index 000000000000..4d6db13cf910
> --- /dev/null
> +++ b/drivers/media/dvb-frontends/cxd2880/cxd2880_io.h
> @@ -0,0 +1,62 @@
> +/*
> + * cxd2880_io.h
> + * Sony CXD2880 DVB-T2/T tuner + demodulator driver
> + * register I/O interface definitions
> + *
> + * Copyright (C) 2016, 2017 Sony Semiconductor Solutions Corporation
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the
> + * Free Software Foundation; version 2 of the License.
> + *
> + * THIS SOFTWARE IS PROVIDED ``AS IS'' AND ANY EXPRESS OR IMPLIED
> + * WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF
> + * MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN
> + * NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT,
> + * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
> + * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
> + * USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
> + * ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
> + * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> + *
> + * You should have received a copy of the GNU General Public License along
> + * with this program; if not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#ifndef CXD2880_IO_H
> +#define CXD2880_IO_H
> +
> +#include "cxd2880_common.h"
> +
> +enum cxd2880_io_tgt {
> +	CXD2880_IO_TGT_SYS,
> +	CXD2880_IO_TGT_DMD
> +};
> +
> +struct cxd2880_io {
> +	enum cxd2880_ret (*read_regs)(struct cxd2880_io *io,
> +				       enum cxd2880_io_tgt tgt, u8 sub_address,
> +				       u8 *data, u32 size);
> +	enum cxd2880_ret (*write_regs)(struct cxd2880_io *io,
> +					enum cxd2880_io_tgt tgt, u8 sub_address,
> +					const u8 *data, u32 size);
> +	enum cxd2880_ret (*write_reg)(struct cxd2880_io *io,
> +				       enum cxd2880_io_tgt tgt, u8 sub_address,
> +				       u8 data);
> +	void *if_object;
> +	u8 i2c_address_sys;
> +	u8 i2c_address_demod;
> +	u8 slave_select;
> +	void *user;
> +};
> +
> +enum cxd2880_ret cxd2880_io_common_write_one_reg(struct cxd2880_io *io,
> +						 enum cxd2880_io_tgt tgt,
> +						 u8 sub_address, u8 data);
> +
> +enum cxd2880_ret cxd2880_io_set_reg_bits(struct cxd2880_io *io,
> +					 enum cxd2880_io_tgt tgt,
> +					 u8 sub_address, u8 data, u8 mask);
> +
> +#endif
> diff --git a/drivers/media/dvb-frontends/cxd2880/cxd2880_stdlib.h b/drivers/media/dvb-frontends/cxd2880/cxd2880_stdlib.h
> new file mode 100644
> index 000000000000..b9ca1b9df110
> --- /dev/null
> +++ b/drivers/media/dvb-frontends/cxd2880/cxd2880_stdlib.h
> @@ -0,0 +1,35 @@
> +/*
> + * cxd2880_stdlib.h
> + * Sony CXD2880 DVB-T2/T tuner + demodulator driver
> + * standard lib function aliases
> + *
> + * Copyright (C) 2016, 2017 Sony Semiconductor Solutions Corporation
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the
> + * Free Software Foundation; version 2 of the License.
> + *
> + * THIS SOFTWARE IS PROVIDED ``AS IS'' AND ANY EXPRESS OR IMPLIED
> + * WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF
> + * MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN
> + * NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT,
> + * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
> + * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
> + * USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
> + * ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
> + * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> + *
> + * You should have received a copy of the GNU General Public License along
> + * with this program; if not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#ifndef CXD2880_STDLIB_H
> +#define CXD2880_STDLIB_H
> +
> +#include <linux/string.h>
> +
> +#define cxd2880_memcpy  memcpy
> +#define cxd2880_memset  memset

No. Just use memcpy()/memset() and get rid of this header.

> +
> +#endif
> diff --git a/drivers/media/dvb-frontends/cxd2880/cxd2880_stopwatch_port.c b/drivers/media/dvb-frontends/cxd2880/cxd2880_stopwatch_port.c
> new file mode 100644
> index 000000000000..14ad6aa6c4c0
> --- /dev/null
> +++ b/drivers/media/dvb-frontends/cxd2880/cxd2880_stopwatch_port.c
> @@ -0,0 +1,71 @@
> +/*
> + * cxd2880_stopwatch_port.c
> + * Sony CXD2880 DVB-T2/T tuner + demodulator driver
> + * time measurement functions
> + *
> + * Copyright (C) 2016, 2017 Sony Semiconductor Solutions Corporation
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the
> + * Free Software Foundation; version 2 of the License.
> + *
> + * THIS SOFTWARE IS PROVIDED ``AS IS'' AND ANY EXPRESS OR IMPLIED
> + * WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF
> + * MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN
> + * NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT,
> + * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
> + * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
> + * USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
> + * ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
> + * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> + *
> + * You should have received a copy of the GNU General Public License along
> + * with this program; if not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include "cxd2880_common.h"
> +
> +#include <linux/ktime.h>
> +#include <linux/time.h>
> +#include <linux/timekeeping.h>
> +
> +static u32 get_time_count(void)
> +{
> +	struct timespec tp;
> +
> +	getnstimeofday(&tp);

As said before, don't use it for timeout checks.

> +
> +	return (u32)((tp.tv_sec * 1000) + (tp.tv_nsec / 1000000));
> +}
> +
> +enum cxd2880_ret cxd2880_stopwatch_start(struct cxd2880_stopwatch *stopwatch)
> +{
> +	if (!stopwatch)
> +		return CXD2880_RESULT_ERROR_ARG;
> +
> +	stopwatch->start_time = get_time_count();
> +
> +	return CXD2880_RESULT_OK;
> +}
> +
> +enum cxd2880_ret cxd2880_stopwatch_sleep(struct cxd2880_stopwatch *stopwatch,
> +					 u32 ms)
> +{
> +	if (!stopwatch)
> +		return CXD2880_RESULT_ERROR_ARG;
> +	CXD2880_ARG_UNUSED(*stopwatch);
> +	CXD2880_SLEEP(ms);
> +
> +	return CXD2880_RESULT_OK;
> +}
> +
> +enum cxd2880_ret cxd2880_stopwatch_elapsed(struct cxd2880_stopwatch *stopwatch,
> +					   u32 *elapsed)
> +{
> +	if (!stopwatch || !elapsed)
> +		return CXD2880_RESULT_ERROR_ARG;
> +	*elapsed = get_time_count() - stopwatch->start_time;
> +
> +	return CXD2880_RESULT_OK;
> +}

You should reimplement the above using jiffies.

Thanks,
Mauro

  parent reply	other threads:[~2017-06-13 12:13 UTC|newest]

Thread overview: 68+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-04-14  1:50 [PATCH v2 0/15] [dt-bindings] [media] Add document file and driver for Sony CXD2880 DVB-T2/T tuner + demodulator Yasunari.Takiguchi
2017-04-14  1:50 ` Yasunari.Takiguchi
     [not found] ` <20170414015043.16731-1-Yasunari.Takiguchi-7U/KSKJipcs@public.gmane.org>
2017-04-14  2:00   ` [PATCH v2 01/15] [dt-bindings] [media] Add document file for CXD2880 SPI I/F Yasunari.Takiguchi-7U/KSKJipcs
2017-04-14  2:00     ` Yasunari.Takiguchi
2017-04-17  4:58     ` Takiguchi, Yasunari
2017-04-17  4:58       ` Takiguchi, Yasunari
2017-04-14  2:08   ` [PATCH v2 02/15] [media] cxd2880-spi: Add support for CXD2008 SPI interface Yasunari.Takiguchi-7U/KSKJipcs
2017-04-14  2:08     ` Yasunari.Takiguchi
     [not found]     ` <20170414020823.17034-1-Yasunari.Takiguchi-7U/KSKJipcs@public.gmane.org>
2017-06-13 11:55       ` Mauro Carvalho Chehab
2017-06-13 11:55         ` Mauro Carvalho Chehab
2017-04-14  2:17   ` [PATCH v2 03/15] [media] cxd2880: Add common files for the driver Yasunari.Takiguchi-7U/KSKJipcs
2017-04-14  2:17     ` Yasunari.Takiguchi
     [not found]     ` <20170414021701.17133-1-Yasunari.Takiguchi-7U/KSKJipcs@public.gmane.org>
2017-06-13 12:13       ` Mauro Carvalho Chehab [this message]
2017-06-13 12:13         ` Mauro Carvalho Chehab
2017-04-14  2:22   ` [PATCH v2 04/15] [media] cxd2880: Add math functions " Yasunari.Takiguchi-7U/KSKJipcs
2017-04-14  2:22     ` Yasunari.Takiguchi
2017-06-13 12:18     ` Mauro Carvalho Chehab
2017-06-13 12:18       ` Mauro Carvalho Chehab
2017-04-14  2:26   ` [PATCH v2 06/15] [media] cxd2880: Add tuner part of " Yasunari.Takiguchi-7U/KSKJipcs
2017-04-14  2:26     ` Yasunari.Takiguchi
     [not found]     ` <20170414022656.17505-1-Yasunari.Takiguchi-7U/KSKJipcs@public.gmane.org>
2017-06-13 13:09       ` Mauro Carvalho Chehab
2017-06-13 13:09         ` Mauro Carvalho Chehab
2017-04-14  2:33   ` [PATCH v2 09/15] [media] cxd2880: Add DVB-T control functions " Yasunari.Takiguchi-7U/KSKJipcs
2017-04-14  2:33     ` Yasunari.Takiguchi
2017-04-14  2:35   ` [PATCH v2 10/15] [media] cxd2880: Add DVB-T monitor and integration layer functions Yasunari.Takiguchi-7U/KSKJipcs
2017-04-14  2:35     ` Yasunari.Takiguchi
2017-04-14  2:40   ` [PATCH v2 12/15] [media] cxd2880: Add DVB-T2 " Yasunari.Takiguchi-7U/KSKJipcs
2017-04-14  2:40     ` Yasunari.Takiguchi
2017-04-14  2:44   ` [PATCH v2 13/15] [media] cxd2880: Add all Makefile files for the driver Yasunari.Takiguchi-7U/KSKJipcs
2017-04-14  2:44     ` Yasunari.Takiguchi
2017-04-14  2:47   ` [PATCH v2 14/15] [media] cxd2880: Add all Kconfig " Yasunari.Takiguchi-7U/KSKJipcs
2017-04-14  2:47     ` Yasunari.Takiguchi
2017-04-14  2:48   ` [PATCH v2 15/15] [media] cxd2880 : Update MAINTAINERS file for CXD2880 driver Yasunari.Takiguchi-7U/KSKJipcs
2017-04-14  2:48     ` Yasunari.Takiguchi
2017-04-14  2:25 ` [PATCH v2 05/15] [media] cxd2880: Add spi device IO routines Yasunari.Takiguchi
2017-04-14  2:25   ` Yasunari.Takiguchi
     [not found]   ` <20170414022523.17417-1-Yasunari.Takiguchi-7U/KSKJipcs@public.gmane.org>
2017-06-13 12:25     ` Mauro Carvalho Chehab
2017-06-13 12:25       ` Mauro Carvalho Chehab
2017-04-14  2:29 ` [PATCH v2 07/15] [media] cxd2880: Add integration layer for the driver Yasunari.Takiguchi
2017-04-14  2:29   ` Yasunari.Takiguchi
2017-04-14  2:31 ` [PATCH v2 08/15] [media] cxd2880: Add top level of " Yasunari.Takiguchi
2017-04-14  2:31   ` Yasunari.Takiguchi
     [not found]   ` <20170414023150.17685-1-Yasunari.Takiguchi-7U/KSKJipcs@public.gmane.org>
2017-06-13 13:30     ` Mauro Carvalho Chehab
2017-06-13 13:30       ` Mauro Carvalho Chehab
2017-06-19  7:56       ` Takiguchi, Yasunari
2017-06-19  7:56         ` Takiguchi, Yasunari
2017-06-23 13:02         ` Mauro Carvalho Chehab
2017-06-25 12:15           ` Mauro Carvalho Chehab
2017-06-26  1:24             ` Mauro Carvalho Chehab
2017-06-27  1:51               ` Takiguchi, Yasunari
2017-06-27  1:51                 ` Takiguchi, Yasunari
2017-04-14  2:38 ` [PATCH v2 11/15] [media] cxd2880: Add DVB-T2 control functions for " Yasunari.Takiguchi
2017-04-14  2:38   ` Yasunari.Takiguchi
2017-04-17  5:09 ` [PATCH v2 0/15] [dt-bindings] [media] Add document file and driver for Sony CXD2880 DVB-T2/T tuner + demodulator Takiguchi, Yasunari
2017-04-17  5:09   ` Takiguchi, Yasunari
     [not found]   ` <5188b958-9a34-4519-5845-a318273592e0-7U/KSKJipcs@public.gmane.org>
2017-05-25  6:15     ` Takiguchi, Yasunari
2017-05-25  6:15       ` Takiguchi, Yasunari
2017-06-01  1:41       ` Takiguchi, Yasunari
2017-06-01  1:41         ` Takiguchi, Yasunari
2017-06-12 13:33         ` Abylay Ospan
     [not found]           ` <CAK3bHNXgbZ0oXbUAYCznpW-iLwQeStFcYRERfRPQcVrk18Pm6g-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-06-13  5:35             ` Takiguchi, Yasunari
2017-06-13  5:35               ` Takiguchi, Yasunari
     [not found]               ` <6586e809-417f-bb4b-5ca5-e38b9641894b-7U/KSKJipcs@public.gmane.org>
2017-06-13  5:39                 ` Abylay Ospan
2017-06-13  5:39                   ` Abylay Ospan
     [not found]       ` <d7c70c53-3fb0-a045-5e1a-1a736bdeda1f-7U/KSKJipcs@public.gmane.org>
2017-06-13 14:38         ` Mauro Carvalho Chehab
2017-06-13 14:38           ` Mauro Carvalho Chehab
2017-06-14  3:01           ` Takiguchi, Yasunari
2017-06-14  3:01             ` Takiguchi, Yasunari

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170613091301.3c8e63a9@vento.lan \
    --to=mchehab-jsyntwtnfakrb7szvlqpia@public.gmane.org \
    --cc=Hideki.Nozawa-7U/KSKJipcs@public.gmane.org \
    --cc=Kota.Yonezawa-7U/KSKJipcs@public.gmane.org \
    --cc=Masayuki.Yamamoto-7U/KSKJipcs@public.gmane.org \
    --cc=Satoshi.C.Watanabe-7U/KSKJipcs@public.gmane.org \
    --cc=Toshihiko.Matsumoto-7U/KSKJipcs@public.gmane.org \
    --cc=Yasunari.Takiguchi-7U/KSKJipcs@public.gmane.org \
    --cc=devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
    --cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-media-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=tbird20d-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.