SuperHouse / esp-open-rtos

Open source FreeRTOS-based ESP8266 software framework
BSD 3-Clause "New" or "Revised" License
1.53k stars 491 forks source link

Improved data bus organization #346

Open Zaltora opened 7 years ago

Zaltora commented 7 years ago

Hi, I would like to standardize the use of data bus. Today, some libraries initialize the bus when else lets the user do it. This becomes confusing and error-prone for the user when using multiple libraries or dynamic usage of component. Initialization of the same bus several times in different places, possible error when the bus is initialized with different settings, need overriden setting when library don't permit it. I want to create a data structure that each library needs to use when specific bus is needed (UART, OneWire, I2C, SPI, ...) This structure would be composed of 3 sub-variables ( just one byte will be used ): -type (Link name like I2C, SPI) -id (the bus identifier 0,1,2 ) -state (bus status: on / off / busy / ... )

Specific device addr or pin will be described into library like some checks. Libraries are not allowed to setting a bus, just use a specific one .

Advantages to do this: -total user control on bus setting. proper initialization. -Bus specific problem independent (busy line problem can be solve for each bus, libraries don't care about it). -Possible to manage an offline bus by libraries .

That just an idea i want discuss about to improve the concept. what did you think and what can be good to do and add ?

vlad-ivanov-name commented 7 years ago

I agree component libraries shouldn't do interface initialization themselves. As for the implementation, I don't think it's necessary to merge all kinds of data buses into a single structure. Current peripheral API (for example, SPI) does quite a good job handling multiple instances nicely with minimal overhead.

busy line problem can be solve for each bus

This kind of problem can be solved by introducing an abstraction layer on top of the existing peripheral API. Thread-safe peripheral access will, admittedly, come with a performance hit. Besides, some users/applications need it and some don't. I can't currently think of the way to easily switch libraries between thread-safe and direct access.

Another thing I thought about is implementing some sort of device tree or module system which would allow to instantiate devices statically and initialize them all at once. Refactoring data bus access will help to achieve that.

Zaltora commented 7 years ago

Okay, no need to regroup all kind of bus. Just a question about SW and HW link. Can be good to link them ? or libraries need to swap between soft and hard ? . Problem for future UART pull request and CS SPI PIN with gpio use. Advantages do not link them is less function call when we need them.

an abstraction layer on top of the existing peripheral API.

Okay, well pretty good idea. the layer can be used to complement the existing one. This layer can add specific status , SW/HW difference, add safe Soft CS pin ?

I can't currently think of the way to easily switch libraries between thread-safe and direct access Idea :

-Multiple Pointer function used by all lib and can be set by one command ( DIRECT / SAFE_THREAD ) (Good for dynamics) -A simple define that allow switch between direct and safe thread ( not in all library just to cahnge a header include that will be safe API ) problem with theses idea that the SAFE API need to be same than DIRECT API. We can still use a define to call a function that dont use argument to remedy.

define test_bus (a,b,c) test_bus(a,b)

I think about update i2C to add like SPI and UART the bus config ( bus 0 and bus 1) to normalize and allow multiple bus .

instantiate devices statically and initialize them all at once

Tell me more about it. User can still control when initialize and what device ?

vlad-ivanov-name commented 7 years ago

Macro abuse is usually not the best way to solve this kind of stuff, I'd go with function pointers.

Tell me more about it. User can still control when initialize and what device?

Well, Linux has this thing called device trees which is used on embedded targets. Suppose you have an ARM device (let's say a tablet); there is a number of peripherals connected to buses like SPI, I2C, I2S and so on — all of these don't have any kind of device detection feature, and peripherals connected to those buses are usually chips soldered on board, so the configuration doesn't change often. So in order to provide a convenient way of telling the Linux kernel about peripheral configuration, device trees are used. Device tree file basically describes the following:

        i2c@101f2000 {
            compatible = "acme,a1234-i2c-bus";
            reg = <0x101f2000 0x1000 >;
            rtc@58 {
                compatible = "maxim,ds1338";
            };
        };

This means there is a memory-mapped I2C controller at (physical) memory address 0x101f2000 and there is an RTC at I2C address 0x58.

Now, the whole system is quite complex (there is a separate compiler for these files). What I thought about is a more or less lightweight alternative: we can instantiate drivers in C and declare addresses/settings at the same time. Like this:

// driver.h
typedef struct {
    void (* init)(void * data);
    void * data;
} driver_t;
// i2c.h
#define I2C_INST(x) x

struct i2c_driver_priv;
typedef struct i2c_driver_priv i2c_driver_priv_t;

typedef struct {
    uint8_t gpio_scl;
    uint8_t gpio_sda;
    // private driver stuff
    i2c_driver_priv_t private;
} i2c_driver_data_t;
// device.h

typedef struct {
    uint8_t i2c;
    uint8_t i2c_addr;
} device_driver_data_t;
// main.c

const driver_t devices[] = {{
    .init = i2c_init,
    .data = (i2c_driver_data_t) {
        .gpio_sda = 1,
        .gpio_scl = 2
    }
}, {
    .init = device_init,
    .data = (device_driver_data_t) {
        .i2c = I2C_INST(0),
        .i2c_addr = 0x56
    }
}};

int main() {
    driver_init(devices);
    return 0;
}

This will require quite a bit of refactoring, but IMO it looks nice and shouldn't affect performance too much.

Zaltora commented 7 years ago

Yes, it is feel like a good idea. I will take a look about this. Thank you @resetnow .

vlad-ivanov-name commented 7 years ago

If you decide to implement something like this, it would make sense to hear from other contributors/maintainers first.

UncleRus commented 7 years ago

I think it must be extra library, not the part of core.

vlad-ivanov-name commented 7 years ago

I think it must be extra library, not the part of core.

Do you mean data bus abstraction layer or device tree?

UncleRus commented 7 years ago

Do you mean data bus abstraction layer or device tree?

Both.

Zaltora commented 7 years ago

If bus abstraction layer is not a part of core. Some library will don't use it and the advantage to got thread safe/direct, busy and others options will disappear no ? device tree can be an extras that will use the bus abstraction layer and component library to work. user will choice to use it or not. For bus abstraction layer, if user set DIRECT option, change will be very minimal. I am wrong ?