Merge request https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/1341 was reviewed by Pavel Pisa
-- Pavel Pisa started a new discussion on cpukit/dev/can/dcan/dcan.c: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/1341#note_153875 > + * completion is currently checked by polling TXRQ. > + */ > + rtems_binary_semaphore_wait_timed_ticks( Please, try without timeouts `rtems_binary_semaphore_wait`. It should work. -- Pavel Pisa started a new discussion on cpukit/dev/can/dcan/dcan.c: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/1341#note_153876 > + internal->frame = ( struct can_frame ){}; > + > + ret = dcan1_read_rx_message_object_2( chip, &internal->frame ); Processing should be done there directly. No `frame_to_pass` required. -- Pavel Pisa started a new discussion on cpukit/dev/can/dcan/dcan.c: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/1341#note_153877 > + * @brief This indicates whether frame contains data to pass upstream. > + */ > + bool frame_to_pass; I think that this is unnecessary. -- Pavel Pisa started a new discussion on cpukit/dev/can/dcan/dcan.c: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/1341#note_153878 > + * Configure BBB pins P9.24 / P9.26 for DCAN1. > + */ > +static void dcan1_pinmux( void ) This should go to AM335x specific file. -- Pavel Pisa started a new discussion on cpukit/dev/can/dcan/dcan.c: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/1341#note_153879 > + * @return 0 on success, negated ETIMEDOUT otherwise. > + */ > +static int dcan1_clock_enable( void ) Move to AM335x specific file. -- Pavel Pisa started a new discussion on cpukit/dev/can/dcan/dcan.c: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/1341#note_153880 > + * @return 0 on success, negated ETIMEDOUT otherwise. > + */ > +static int dcan1_leave_init_mode( struct rtems_can_chip *chip ) It should start by `dcan_` not `dcan1_`, there should not be anything specific to individual interface. -- Pavel Pisa started a new discussion on cpukit/dev/can/dcan/dcan.c: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/1341#note_153881 > +} > + > +static int dcan_set_bittiming( Seems to be unnecessary forward declaration. -- Pavel Pisa started a new discussion on cpukit/dev/can/dcan/dcan.c: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/1341#note_153882 > + > + > +static bool dcan_tx_mo1_idle( struct rtems_can_chip *chip ) static bool dcan_tx_mo_is_idle( struct rtems_can_chip *chip, int obj ) -- Pavel Pisa started a new discussion on cpukit/dev/can/dcan/dcan.c: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/1341#note_153883 > + * @return 0 on success, negated errno otherwise. > + */ > +static int dcan1_write_frame_mo1( ``` static int dcan_mo_write_frame( struct rtems_can_chip *chip, int mobj, const struct can_frame *frame ) ``` -- Pavel Pisa started a new discussion on cpukit/dev/can/dcan/dcan.c: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/1341#note_153884 > + ); > + > + /* Check for Tx result, finished OK or error. If error then add information about error to the `rtems_can_queue_filter_frame_to_edges`, ``` rtems_can_queue_filter_frame_to_edges( &chip->qends_dev->base, txb_info->edge, &txb_info->slot->frame, CAN_FRAME_TXERR ); ``` -- Pavel Pisa started a new discussion on cpukit/dev/can/dcan/dcan.c: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/1341#note_153885 > +* > +*/ > +static int dcan_configure_tx( struct rtems_can_chip *chip ) Versatile to choose which message objects -- Pavel Pisa started a new discussion on cpukit/dev/can/dcan/dcan.c: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/1341#note_153886 > + > +/** Configure DCAN1 RX Message Object 2*/ > +static int dcan_configure_rx( struct rtems_can_chip *chip ) Versatile to choose which message objects -- Pavel Pisa started a new discussion on cpukit/dev/can/dcan/dcan.c: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/1341#note_153887 > + * @return 1 on success, negated errno otherwise. > + */ > +static int dcan1_read_rx_message_object_2( ``` static int dcan_mo_read_rx_message( struct rtems_can_chip *chip, int mobj, struct can_frame *frame ) ``` -- Pavel Pisa started a new discussion on cpukit/dev/can/dcan/dcan.c: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/1341#note_153888 > +} > + > +static struct can_frame *dcan_get_rx_frame( struct rtems_can_chip *chip ) Probably not required when Rx moved out of interrupt handler -- Pavel Pisa started a new discussion on cpukit/dev/can/dcan/dcan.c: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/1341#note_153889 > +static void dcan_enable_if3_update_for_rx_msg_obj( > + struct rtems_can_chip *chip > +) IF3 use only for DMA, use IF1 or 2 for all other. -- Pavel Pisa started a new discussion on cpukit/dev/can/dcan/dcan.c: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/1341#note_153890 > + CAN_STATE_ERROR_ACTIVE > + ); > + There should be protection by mutex ``` /* Mark HW as configured */ rtems_binary_semaphore_try_wait( &chip->stop_sem ); rtems_can_set_bit( RTEMS_CAN_CHIP_RUNNING, &chip->flags ); rtems_can_stats_reset( &chip->chip_stats ); rtems_can_stats_set_state( &chip->chip_stats, CAN_STATE_ERROR_ACTIVE ); start_chip_unlock: rtems_mutex_unlock( &chip->lock ); return ret; ``` -- Pavel Pisa started a new discussion on cpukit/dev/can/dcan/dcan.c: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/1341#note_153891 > +static int dcan_start_chip( struct rtems_can_chip *chip ) > +{ > + struct rtems_can_bittiming bt = { 0 }; ``` rtems_mutex_lock( &chip->lock ); if ( rtems_can_test_bit( RTEMS_CAN_CHIP_RUNNING, &chip->flags ) ) { /* Chip was already configured, skip */ rtems_mutex_unlock( &chip->lock ); return 0; } ``` -- Pavel Pisa started a new discussion on cpukit/dev/can/dcan/dcan.c: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/1341#note_153892 > + dcan_write_reg( internal, DCAN_CTL, ctl ); > + > + return 0; ``` rtems_can_stats_set_state( &chip->chip_stats, CAN_STATE_STOPPED ); rtems_can_bus_notify_chip_stop( chip ); rtems_mutex_unlock( &chip->lock ); ``` -- Pavel Pisa started a new discussion on cpukit/dev/can/dcan/dcan.c: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/1341#note_153893 > + struct dcan_internal *internal = chip->internal; > + uint32_t ctl; > + ``` rtems_mutex_lock( &chip->lock ); if ( rtems_can_test_bit( RTEMS_CAN_CHIP_RUNNING, &chip->flags ) == 0 ) { /* Already stopped, return. */ rtems_mutex_unlock( &chip->lock ); return 0; } rtems_can_clear_bit( RTEMS_CAN_CHIP_RUNNING, &chip->flags ); /* disable interrupts there */ rtems_binary_semaphore_post( &chip->qends_dev->worker_sem ); ``` Check code in SJA1000 or CTU CAN FD for start and stop concurrent calls protection https://gitlab.rtems.org/rtems/rtos/rtems/-/blob/main/cpukit/dev/can/ctucanfd/ctucanfd.c?ref_type=heads#L1562 -- Pavel Pisa started a new discussion on cpukit/include/dev/can/dcan.h: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/1341#note_153894 > + (*(volatile uint32_t*)(addr)) // need to change to inline function later > + > +#define SOC_CM_PER_REGS 0x44E00000u All AM335x specific values should go to other file, i.e. under BSP or `dcan_am335x.h` or similar. -- View it on GitLab: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/1341 You're receiving this email because of your account on gitlab.rtems.org. Unsubscribe from this thread: https://gitlab.rtems.org/-/sent_notifications/4-9ldfrfo4qyjxiix1bqxyan4gp-1d/unsubscribe | Manage all notifications: https://gitlab.rtems.org/-/profile/notifications | Help: https://gitlab.rtems.org/help
_______________________________________________ bugs mailing list [email protected] http://lists.rtems.org/mailman/listinfo/bugs
