Re: [PATCH net-next v1 01/13] net: wwan: tmi: Add PCIe core

[Sergey Ryazanov]

Hello Yanchao,

On 07.12.2022 06:33, Yanchao Yang (杨彦超) wrote:
> On Sun, 2022-12-04 at 22:52 +0400, Sergey Ryazanov wrote:
> > On 22.11.2022 15:11, Yanchao Yang wrote:
> > > Registers the TMI device driver with the kernel. Set up all the
> > > fundamental
> > > configurations for the device: PCIe layer, Modem Host Cross Core
> > > Interface
> > > (MHCCIF), Reset Generation Unit (RGU), modem common control
> > > operations and
> > > build infrastructure.
> > >
> > > * PCIe layer code implements driver probe and removal, MSI-X
> > > interrupt
> > > initialization and de-initialization, and the way of resetting the
> > > device.
> > > * MHCCIF provides interrupt channels to communicate events such as
> > > handshake,
> > > PM and port enumeration.
> > > * RGU provides interrupt channels to generate notifications from
> > > the device
> > > so that the TMI driver could get the device reset.
> > > * Modem common control operations provide the basic read/write
> > > functions of
> > > the device's hardware registers, mask/unmask/get/clear functions of
> > > the
> > > device's interrupt registers and inquiry functions of the device's
> > > status.
> > >
> > > Signed-off-by: Ting Wang <ting.wang@xxxxxxxxxxxx>
> > > Signed-off-by: MediaTek Corporation <linuxwwan@xxxxxxxxxxxx>
> > > ---
> > >    drivers/net/wwan/Kconfig                 |   11 +
> > >    drivers/net/wwan/Makefile                |    1 +
> > >    drivers/net/wwan/mediatek/Makefile       |   12 +
> > >    drivers/net/wwan/mediatek/mtk_common.h   |   30 +
> > >    drivers/net/wwan/mediatek/mtk_dev.c      |   50 +
> > >    drivers/net/wwan/mediatek/mtk_dev.h      |  503 ++++++++++
> > >    drivers/net/wwan/mediatek/pcie/mtk_pci.c | 1164
> > > ++++++++++++++++++++++
> > >    drivers/net/wwan/mediatek/pcie/mtk_pci.h |  150 +++
> > >    drivers/net/wwan/mediatek/pcie/mtk_reg.h |   69 ++
> > >    9 files changed, 1990 insertions(+)
> > >    create mode 100644 drivers/net/wwan/mediatek/Makefile
> > >    create mode 100644 drivers/net/wwan/mediatek/mtk_common.h
> > >    create mode 100644 drivers/net/wwan/mediatek/mtk_dev.c
> > >    create mode 100644 drivers/net/wwan/mediatek/mtk_dev.h
> > >    create mode 100644 drivers/net/wwan/mediatek/pcie/mtk_pci.c
> > >    create mode 100644 drivers/net/wwan/mediatek/pcie/mtk_pci.h
> > >    create mode 100644 drivers/net/wwan/mediatek/pcie/mtk_reg.h
> > >
> > > diff --git a/drivers/net/wwan/Kconfig b/drivers/net/wwan/Kconfig
> > > index 3486ffe94ac4..a93a0c511d50 100644
> > > --- a/drivers/net/wwan/Kconfig
> > > +++ b/drivers/net/wwan/Kconfig
> > > @@ -119,6 +119,17 @@ config MTK_T7XX
> > >    
> > >    	  If unsure, say N.
> > >    
> > > +config MTK_TMI
> > > +	tristate "TMI Driver for Mediatek T-series Device"
> > > +	depends on PCI
> > > +	help
> > > +	  This driver enables Mediatek T-series WWAN Device
> > > communication.
> > > +
> > > +	  If you have one of those Mediatek T-series WWAN Modules and
> > > wish to
> > > +	  use it in Linux say Y/M here.
> >
> > From this and the series descriptions, it is unclear which modem
> > chips this driver is intended for and how does it correlate with the
> > T7xx driver? Is the TMI driver a drop-in replacement for the t7xx driver,
> > or does the TMI driver support any T-series chips except t7xx?
> > The driver is intended for t8xx or later T-series modem chips in the
> future. Currently, t7xx is not support.

Can you add this information to the option description to make it easier 
for users to choose?

BTW, just curious, do you have any plans to add T7xx support to the TMI 
driver, or maybe merge them or factor out the common code into a common 
library? I am asking because I noticed some common code and modem 
components, but that is not addressed in the cover letter. Or is this 
feeling misleading and these two series are very different?

> > > +
> > > +	  If unsure, say N.
> > > +
> > >    endif # WWAN
> > >    
> > >    endmenu
> > > diff --git a/drivers/net/wwan/Makefile b/drivers/net/wwan/Makefile
> > > index 3960c0ae2445..198d8074851f 100644
> > > --- a/drivers/net/wwan/Makefile
> > > +++ b/drivers/net/wwan/Makefile
> > > @@ -14,3 +14,4 @@ obj-$(CONFIG_QCOM_BAM_DMUX) += qcom_bam_dmux.o
> > >    obj-$(CONFIG_RPMSG_WWAN_CTRL) += rpmsg_wwan_ctrl.o
> > >    obj-$(CONFIG_IOSM) += iosm/
> > >    obj-$(CONFIG_MTK_T7XX) += t7xx/
> > > +obj-$(CONFIG_MTK_TMI) += mediatek/
> >
> > The driver is called mtk_tmi, but its code is placed to the
> > directory
> > with too generic name 'mediatek'. Do you plan too keep all possible
> > future drivers in this directory? >
> Yes, we plan to put all mediatek's wwan driver into the same directory.
> Currently, there is only T-series modem driver. So we don't create
> 'tmi' folder under 'mediatek' directory explicitly.

Thank you for the clarification.

--
Sergey