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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Wed, 26 Feb 2020 11:55:48 +0000
From:   Lee Jones <lee.jones@...aro.org>
To:     Tony Lindgren <tony@...mide.com>
Cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Alan Cox <gnomes@...rguk.ukuu.org.uk>,
        Jiri Slaby <jslaby@...e.cz>, Johan Hovold <johan@...nel.org>,
        Merlijn Wajer <merlijn@...zup.org>,
        Pavel Machek <pavel@....cz>,
        Peter Hurley <peter@...leysoftware.com>,
        Rob Herring <robh@...nel.org>,
        Sebastian Reichel <sre@...nel.org>,
        linux-serial@...r.kernel.org, devicetree@...r.kernel.org,
        linux-omap@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/4] mfd: motmdm: Add Motorola TS 27.010 serdev modem
 driver for droid4

On Thu, 20 Feb 2020, Tony Lindgren wrote:

> Many Motorola phones are controlling the modem using a custom variant
> of TS 27.010 serial line discipline. Devices on these modems have a
> dedicated TS 27.010 channel for features like audio mixer, GNSS, voice
> modem, SIM card reader and so on.
> 
> This driver allows using various devices on the modem. In order to do
> that, we need to take care of the following three things:
> 
> 1. Provide /dev/motmdm* character devices for apps to use for talking
>    to the various devices on the modem
> 
> 2. Handle Motorola custom protocol over TS 27.010 to make the channels
>    usable for userspace
> 
> 3. Coordinate PM runtime with the USB PHY because of shared GPIO pins
>    with the USB PHY
> 
> With this patch, folks with droid4 can place a voice call with just:
> 
> $ printf "ATD%s,0\r" "${phone_number}" > /dev/motmdm1
> D:OK
> ~+CIEV=1,1,0
> ...
> $ printf "ATH\r" > /dev/motmdm1
> H:OK
> 
> Also SMS can be sent with this patch using /dev/motmdm3 for sending,
> and /dev/motmdm9 for receiving messages.
> 
> Note that the audio mixer needs additional patches though. I will be
> sending those as a separate series of patches.
> 
> Signed-off-by: Tony Lindgren <tony@...mide.com>
> ---
>  drivers/mfd/Kconfig        |    9 +
>  drivers/mfd/Makefile       |    1 +
>  drivers/mfd/motorola-mdm.c | 1200 ++++++++++++++++++++++++++++++++++++

I'm not even going to start reviewing this as I can see, without even
looking at the code, that this has too much functionality (stuff that
does stuff) contained.

Please move as much functionality out into the subsystems as
possible.  Ideally, MFDs should be responsible for obtaining and
registering shared resources and registering child devices.  Anything
else should be shifted out to an appropriate subsystem.

MFD is not Misc.

>  3 files changed, 1210 insertions(+)
>  create mode 100644 drivers/mfd/motorola-mdm.c

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Powered by blists - more mailing lists