lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Thu, 6 Feb 2020 10:08:00 -0700 From: Jeffrey Hugo <jhugo@...eaurora.org> To: Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>, gregkh@...uxfoundation.org, arnd@...db.de Cc: smohanad@...eaurora.org, kvalo@...eaurora.org, bjorn.andersson@...aro.org, hemantk@...eaurora.org, linux-arm-msm@...r.kernel.org, linux-kernel@...r.kernel.org Subject: Re: [PATCH v2 02/16] bus: mhi: core: Add support for registering MHI controllers On 1/31/2020 6:49 AM, Manivannan Sadhasivam wrote: > This commit adds support for registering MHI controller drivers with > the MHI stack. MHI controller drivers manages the interaction with the > MHI client devices such as the external modems and WiFi chipsets. They > are also the MHI bus master in charge of managing the physical link > between the host and client device. > > This is based on the patch submitted by Sujeev Dias: > https://lkml.org/lkml/2018/7/9/987 > > Signed-off-by: Sujeev Dias <sdias@...eaurora.org> > Signed-off-by: Siddartha Mohanadoss <smohanad@...eaurora.org> > [jhugo: added static config for controllers and fixed several bugs] > Signed-off-by: Jeffrey Hugo <jhugo@...eaurora.org> > [mani: removed DT dependency, splitted and cleaned up for upstream] > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org> > +/** > + * enum mhi_device_type - Device types > + * @MHI_DEVICE_XFER: Handles data transfer > + * @MHI_DEVICE_TIMESYNC: Use for timesync feature > + * @MHI_DEVICE_CONTROLLER: Control device > + */ > +enum mhi_device_type { > + MHI_DEVICE_XFER, > + MHI_DEVICE_TIMESYNC, > + MHI_DEVICE_CONTROLLER, > +}; Time sync support was removed, no? I don't see MHI_DEVICE_TIMESYNC used anywhere in the code. > +/** > + * enum mhi_er_data_type - Event ring data types > + * @MHI_ER_DATA: Only client data over this ring > + * @MHI_ER_CTRL: MHI control data and client data > + * @MHI_ER_TSYNC: Time sync events > + */ > +enum mhi_er_data_type { > + MHI_ER_DATA, > + MHI_ER_CTRL, > + MHI_ER_TSYNC, > +}; Time sync support was removed, no? I don't see MHI_ER_TSYNC used anywhere in the code. > +/** > + * struct mhi_controller - Master MHI controller structure > + * @dev: Driver model device node for the controller (required) > + * @mhi_dev: MHI device instance for the controller > + * @regs: Base address of MHI MMIO register space (required) > + * @iova_start: IOMMU starting address for data (required) > + * @iova_stop: IOMMU stop address for data (required) > + * @fw_image: Firmware image name for normal booting (required) > + * @edl_image: Firmware image name for emergency download mode (optional) > + * @sbl_size: SBL image size downloaded through BHIe (optional) > + * @seg_len: BHIe vector size (optional) > + * @mhi_chan: Points to the channel configuration table > + * @lpm_chans: List of channels that require LPM notifications > + * @irq: base irq # to request (required) > + * @max_chan: Maximum number of channels the controller supports > + * @total_ev_rings: Total # of event rings allocated > + * @hw_ev_rings: Number of hardware event rings > + * @sw_ev_rings: Number of software event rings > + * @nr_irqs_req: Number of IRQs required to operate (optional) > + * @nr_irqs: Number of IRQ allocated by bus master (required) > + * @mhi_event: MHI event ring configurations table > + * @mhi_cmd: MHI command ring configurations table > + * @mhi_ctxt: MHI device context, shared memory between host and device > + * @pm_mutex: Mutex for suspend/resume operation > + * @pm_lock: Lock for protecting MHI power management state > + * @timeout_ms: Timeout in ms for state transitions > + * @pm_state: MHI power management state > + * @db_access: DB access states > + * @ee: MHI device execution environment > + * @dev_wake: Device wakeup count > + * @pending_pkts: Pending packets for the controller > + * @transition_list: List of MHI state transitions > + * @transition_lock: Lock for protecting MHI state transition list > + * @wlock: Lock for protecting device wakeup > + * @st_worker: State transition worker > + * @fw_worker: Firmware download worker > + * @syserr_worker: System error worker > + * @state_event: State change event > + * @status_cb: CB function to notify power states of the device (required) > + * @link_status: CB function to query link status of the device (required) > + * @wake_get: CB function to assert device wake (optional) > + * @wake_put: CB function to de-assert device wake (optional) > + * @wake_toggle: CB function to assert and de-assert device wake (optional) > + * @runtime_get: CB function to controller runtime resume (required) > + * @runtimet_put: CB function to decrement pm usage (required) > + * @buffer_len: Bounce buffer length > + * @bounce_buf: Use of bounce buffer > + * @fbc_download: MHI host needs to do complete image transfer (optional) > + * @pre_init: MHI host needs to do pre-initialization before power up > + * @wake_set: Device wakeup set flag > + */ I'm happy the optional/required is listed. However if I look at this from the perspective of someone writing a controller for the first time, I'm not confident the required/optional annotations are clear enough. Perhaps a quick sentance explaining that those annotations indicate fields which must be populated prior to mhi_register_controller() ? > + > +/** > + * struct mhi_device - Structure representing a MHI device which binds > + * to channels > + * @id: Pointer to MHI device ID struct > + * @chan_name: Name of the channel to which the device binds > + * @mhi_cntrl: Controller the device belongs to > + * @ul_chan: UL channel for the device > + * @dl_chan: DL channel for the device > + * @dev: Driver model device node for the MHI device > + * @dev_type: MHI device type > + * @tiocm: Device current terminal settings > + * @dev_wake: Device wakeup counter > + */ > +struct mhi_device { > + const struct mhi_device_id *id; > + const char *chan_name; > + struct mhi_controller *mhi_cntrl; > + struct mhi_chan *ul_chan; > + struct mhi_chan *dl_chan; > + struct device dev; > + enum mhi_device_type dev_type; > + u32 tiocm; This was for the old ioctl support, right? I don't see it used anywhere. > + u32 dev_wake; > +}; -- Jeffrey Hugo Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
Powered by blists - more mailing lists