Hello Martin, Am 10.01.19 um 17:37 schrieb kernel@xxxxxxxxxxxxxxxx: > Hi Wolfgang, > >> On 10.01.2019, at 09:30, Wolfgang Grandegger <wg@xxxxxxxxxxxxxx> wrote: >> >> Hello Martin, >> >> Do I understand you correctly, this is the basic driver and the >> optimizations are not yet included!? Well, I expected less code than >> last time but now it's spit up in smaller files, at least. Will try to >> do a review this week… >> > This is the basic driver - split into the 3 main components: > * core (essentially basic module management, identification and clock/ram) > * GPIO handling of the GPIO/INT pins > * CAN > > where CAN is obviously the biggest, but it is also split out into > * basic > * interrupt handling > * fifo setup > * CAN Frame reception > * CAN Frame transmission > > The place that we could split out more is reducing the fifo-setup > to a single TX and RX fifo, but the cost of ripping out that portion > (and itsstructural dependencies) is quite high just to produce code > that is never actually run… > > So I have made the driver all minimal removing all the big performance > optimizations while leaving the basic multi-fifo setup in the config > - mostly in mcp25xxfd_can_fifo and some in mcp25xxfd_can_rx and > mcp25xxfd_can_tx. > > I have to admit that there is one optimization that is in can_tx that > could get separated out into a separate patch. > > That is the function mcp25xxfd_can_int_handle_tefif_oportunistic > the removal of which would simplify mcp25xxfd_can_int_handle_tefif > slightly, but that code should be fairly simple to read. > > If you want, then I can move this to a separate patch in the next > version. > > Basic setup, interrupt handling, SERR error handling and others > would still need to happen in the initial driver, so that is a > big chunk of the overall driver. > > There is already a new patchset in the pipeline that: > * addresses some concerns in mcp_write_then_* with regards to the > use of stack variables for transmit/receive buffers > - rewrite of function. > (reported by Enrico Scholz) > * initialization of 2 variables in priv (reported by Enrico Scholz). > * adds 4 optimization patches: > * can: mcp25xxfd: optimize tef reads reading multiple TEFs in one go > * can: mcp25xxfd: optimize SPI reads of FIFOs in can2.0 mode > * can: mcp25xxfd: add prediction of CanFD frames sizes based on history > * can: mcp25xxfd: optimize reception of big CanFD frame reception with BRS > > There is a bug with regards to transmission of CanFD with BRS that > still needs to get fixed before I send out the next version - what > is the root cause here is unclear (this may be a regression in the > calculation of data bitrate that gives now different SJW values than > before that impacts the controllers ability to send BRS frames, > but it could also be something else like TDC (= Transmitter Delay > Compensation). > > But I guess I shall also wait for further feedback before sending > out this next version. Yes, please wait for my review! And there is the DEBUG_FS stuff increasing code size as well. We should start with *one* robust and efficient solution working for most CAN use-cases. Wolfgang.
