[PATCH 3/5] Add rtems i2c adaptation layer

[Christian Mauderer]

On 01/06/2019 21:20, Vijay Kumar Banerjee wrote:
> ---
>  libbsd.py                |   1 +
>  rtemsbsd/i2c/rtems-i2c.c | 218 +++++++++++++++++++++++++++++++++++++++
>  2 files changed, 219 insertions(+)
>  create mode 100644 rtemsbsd/i2c/rtems-i2c.c

I think the file would be better located at
    rtemsbsd/sys/dev/iicbus/rtems-i2c.c

The directory structure for drivers should be similar to the one in FreeBSD.

> 
> diff --git a/libbsd.py b/libbsd.py
> index a25d3a8a..6e8ff987 100644
> --- a/libbsd.py
> +++ b/libbsd.py
> @@ -771,6 +771,7 @@ class iic(builder.Module):
>          self.addRTEMSSourceFiles(
>              [
>                  'local/iicbus_if.c',
> +                'i2c/rtems-i2c.c',
>              ],
>              mm.generator['source']()
>          )
> diff --git a/rtemsbsd/i2c/rtems-i2c.c b/rtemsbsd/i2c/rtems-i2c.c
> new file mode 100644
> index 00000000..5189e5fe
> --- /dev/null
> +++ b/rtemsbsd/i2c/rtems-i2c.c
> @@ -0,0 +1,218 @@
> +/*
> + * Copyright (c) 2019 Vijay Kumar Banerjee <vijaykumar9597 at gmail.com>.
> + * All rights reserved.
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions
> + * are met:
> + * 1. Redistributions of source code must retain the above copyright
> + *    notice, this list of conditions and the following disclaimer.
> + * 2. Redistributions in binary form must reproduce the above copyright
> + *    notice, this list of conditions and the following disclaimer in the
> + *    documentation and/or other materials provided with the distribution.
> + *
> + * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``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 OR CONTRIBUTORS 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.
> + */
> +
> +#include <machine/rtems-bsd-kernel-space.h>
> +
> +#include <stdlib.h>
> +#include <stdio.h>
> +#include <string.h>
> +#include <fcntl.h>
> +
> +#include <sys/module.h>
> +#include <sys/param.h>
> +#include <sys/kernel.h>
> +#include <sys/errno.h>
> +#include <sys/bus.h>
> +
> +#include <dev/iicbus/iicbus.h>
> +#include <dev/iicbus/iic.h>
> +#include <dev/iicbus/iiconf.h>
> +#include <dev/ofw/ofw_bus.h>
> +#include <dev/ofw/ofw_bus_subr.h>
> +#include <dev/ofw/openfirm.h>
> +
> +#include <rtems/bsd/local/iicbus_if.h>
> +
> +#include <linux/i2c.h>
> +#include <linux/i2c-dev.h>
> +
> +typedef struct i2c_msg i2c_msg;
> +
> +struct i2c_softc {
> +	device_t dev;
> +	device_t sc_iicbus;
> +	char *path;
> +	int fd;
> +	int mem_rid;
> +	int irq_rid;

mem_rid and irq_rid seem to be unused.

> +};
> +
> +static int
> +rtems_i2c_probe(device_t dev)
> +{
> +	if (!ofw_bus_status_okay(dev)) {
> +		return (ENXIO);
> +	}
> +	if (!ofw_bus_is_compatible(dev, "rtems,bsp-i2c")){
> +		return (ENXIO);
> +	}
> +
> +	device_set_desc(dev, "RTEMS libbsd I2C");
> +	return (BUS_PROBE_SPECIFIC);
> +}
> +
> +static int
> +rtems_i2c_attach(device_t dev)
> +{
> +	phandle_t node;
> +	ssize_t compatlen;
> +	char *compat;
> +	char *curstr;
> +	struct i2c_softc *sc;
> +	int len;
> +
> +	sc = device_get_softc(dev);
> +	sc->dev = dev;
> +	node = ofw_bus_get_node(sc->dev);
> +
> +	len = OF_getprop_alloc(node, "rtems,i2c-path", &sc->path);
> +	if (len == -1){
> +		device_printf(sc->dev, "Path not found in Device Tree");
> +		OF_prop_free(sc->path);
> +		return (ENXIO);
> +	}
> +	else{

You don't really need the else here. If your error case hits, you return
anyway. So you can save one level of indentation for the next block.

> +		if ((sc->sc_iicbus = device_add_child(sc->dev, "iicbus", -1)) == NULL) {
> +			device_printf(sc->dev, "could not allocate iicbus instance\n");
> +			OF_prop_free(sc->path);
> +			return (ENXIO);
> +		}
> +		config_intrhook_oneshot((ich_func_t)bus_generic_attach, sc->dev);
> +	}
> +	return (0);
> +}
> +
> +static int
> +rtems_i2c_detach(device_t dev)
> +{
> +	struct i2c_softc *sc;
> +	int error;
> +
> +	sc = device_get_softc(dev);
> +
> +	OF_prop_free(sc->path);
> +
> +	if (sc->sc_iicbus && (error = device_delete_child(dev, sc->sc_iicbus)) != 0)
> +		return (error);
> +
> +	if ((error = bus_generic_detach(sc->dev)) != 0) {
> +		device_printf(sc->dev, "cannot detach child devices\n");
> +		return (error);
> +	}

Maybe we misunderstood each other there in our chat: The detach should
have roughly a reverse order of the attach. The last action in your
attach is the bus_generic_attach so the first one in detach should be
the bus_generic_detach.

> +
> +
> +	return (0);
> +}
> +
> +static int
> +rtems_i2c_transfer(device_t dev, struct iic_msg *msgs, u_int num)
> +{
> +	i2c_msg *messages;
> +	int err;
> +	char *addr;
> +	struct i2c_softc *sc;
> +	struct i2c_rdwr_ioctl_data ioctl_data;
> +
> +	sc = device_get_softc(dev);
> +
> +	/* Open /dev/iic0 */
> +	sc->fd = open(sc->path, O_RDWR);

Why save the fd in the softc if you only use it in this function? You
can use a local variable here.

> +	if (sc->fd < 0) {
> +		device_printf(sc->dev, "%s\n", strerror(errno));
> +		return errno;
> +	}
> +
> +	/* cast iic_msg to i2c_msg */
> +	messages = (i2c_msg *) msgs;
> +	ioctl_data.msgs = messages;
> +	ioctl_data.nmsgs = num;
> +
> +	/* IOCTL call to write */
> +	err = ioctl(sc->fd, I2C_RDWR, &ioctl_data);
> +	if (err < 0){
> +		device_printf(sc->dev, "%s\n", strerror(errno));
> +		close(sc->fd);
> +		return errno;
> +	}
> +
> +	/* Close the device */
> +	close(sc->fd);
> +
> +	return (0);
> +}
> +
> +static int
> +rtems_i2c_reset(device_t dev, u_char speed, u_char addr, u_char *oldaddr)
> +{
> +	return (0);
> +}
> +
> +static phandle_t
> +rtems_i2c_get_node(device_t bus, device_t dev)
> +{
> +
> +	/* Share controller node with iibus device. */
> +	return (ofw_bus_get_node(bus));
> +}
> +
> +static device_method_t rtems_i2c_methods[] = {
> +	/* Device Interface */
> +	DEVMETHOD(device_probe,		rtems_i2c_probe),
> +	DEVMETHOD(device_attach,	rtems_i2c_attach),
> +	DEVMETHOD(device_detach,	rtems_i2c_detach),
> +
> +	/* Bus interface */
> +	DEVMETHOD(bus_setup_intr,	bus_generic_setup_intr),
> +	DEVMETHOD(bus_teardown_intr,	bus_generic_teardown_intr),
> +	DEVMETHOD(bus_alloc_resource,	bus_generic_alloc_resource),
> +	DEVMETHOD(bus_release_resource, bus_generic_release_resource),
> +	DEVMETHOD(bus_activate_resource, bus_generic_activate_resource),
> +	DEVMETHOD(bus_deactivate_resource, bus_generic_deactivate_resource),
> +	DEVMETHOD(bus_adjust_resource,	bus_generic_adjust_resource),
> +	DEVMETHOD(bus_set_resource, bus_generic_rl_set_resource),
> +	DEVMETHOD(bus_get_resource, bus_generic_rl_get_resource),
> +
> +	/* OFW methods */
> +	DEVMETHOD(ofw_bus_get_node, rtems_i2c_get_node),
> +
> +	/* iicbus interface */
> +	DEVMETHOD(iicbus_callback,	iicbus_null_callback),
> +	DEVMETHOD(iicbus_reset,		rtems_i2c_reset),
> +	DEVMETHOD(iicbus_transfer,	rtems_i2c_transfer),
> +
> +	DEVMETHOD_END
> +};
> +
> +static driver_t rtems_i2c_driver = {
> +	"rtems_i2c",
> +	rtems_i2c_methods,
> +	sizeof(struct i2c_softc),
> +};
> +
> +static devclass_t rtems_i2c_devclass;
> +DRIVER_MODULE(rtems_i2c, simplebus, rtems_i2c_driver, rtems_i2c_devclass, 0, 0);
> +DRIVER_MODULE(iicbus, rtems_i2c, iicbus_driver, iicbus_devclass, 0, 0);
> +DRIVER_MODULE(ofw_iicbus, rtems_i2c, ofw_iicbus_driver, ofw_iicbus_devclass, 0, 0);
>