Linux Serial subsystem development
 help / color / mirror / Atom feed
* [PATCH 4/6] spi: spi-geni-qcom: Add interconnect support
From: Alok Chauhan @ 2019-01-22  6:33 UTC (permalink / raw)
  To: linux-arm-msm, devicetree, linux-kernel, linux-i2c, linux-spi,
	linux-serial, Mark Brown
  Cc: andy.gross, david.brown, georgi.djakov, dianders, swboyd,
	bjorn.andersson, Alok Chauhan
In-Reply-To: <1548138816-1149-1-git-send-email-alokc@codeaurora.org>

Get the interconnect paths for SPI based Serial Engine device
and vote accordingly based on maximum supported SPI frequency.

Signed-off-by: Alok Chauhan <alokc@codeaurora.org>
---
 drivers/spi/spi-geni-qcom.c | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/drivers/spi/spi-geni-qcom.c b/drivers/spi/spi-geni-qcom.c
index fdb7cb88..7bbbe9d 100644
--- a/drivers/spi/spi-geni-qcom.c
+++ b/drivers/spi/spi-geni-qcom.c
@@ -12,6 +12,7 @@
 #include <linux/qcom-geni-se.h>
 #include <linux/spi/spi.h>
 #include <linux/spinlock.h>
+#include <linux/interconnect.h>
 
 /* SPI SE specific registers and respective register fields */
 #define SE_SPI_CPHA		0x224
@@ -589,6 +590,15 @@ static int spi_geni_probe(struct platform_device *pdev)
 	spin_lock_init(&mas->lock);
 	pm_runtime_enable(&pdev->dev);
 
+	/* Set the bus quota to a reasonable value */
+	mas->se.avg_bw = Bps_to_icc(2500);
+	mas->se.peak_bw = Bps_to_icc(200000000);
+	ret = geni_interconnect_init(&mas->se);
+	if (ret) {
+		dev_err(&pdev->dev, "interconnect_init failed %d\n", ret);
+		return ret;
+	}
+
 	ret = spi_geni_init(mas);
 	if (ret)
 		goto spi_geni_probe_runtime_disable;
@@ -628,8 +638,15 @@ static int __maybe_unused spi_geni_runtime_suspend(struct device *dev)
 {
 	struct spi_master *spi = dev_get_drvdata(dev);
 	struct spi_geni_master *mas = spi_master_get_devdata(spi);
+	int ret;
 
-	return geni_se_resources_off(&mas->se);
+	ret = geni_se_resources_off(&mas->se);
+	if (ret)
+		return ret;
+
+	geni_icc_update_bw(&mas->se, false);
+
+	return 0;
 }
 
 static int __maybe_unused spi_geni_runtime_resume(struct device *dev)
@@ -637,6 +654,7 @@ static int __maybe_unused spi_geni_runtime_resume(struct device *dev)
 	struct spi_master *spi = dev_get_drvdata(dev);
 	struct spi_geni_master *mas = spi_master_get_devdata(spi);
 
+	geni_icc_update_bw(&mas->se, true);
 	return geni_se_resources_on(&mas->se);
 }
 
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply related

* [PATCH 3/6] i2c: i2c-qcom-geni: Add interconnect support
From: Alok Chauhan @ 2019-01-22  6:33 UTC (permalink / raw)
  To: linux-arm-msm, devicetree, linux-kernel, linux-i2c, linux-spi,
	linux-serial, Alok Chauhan, Karthikeyan Ramasubramanian
  Cc: andy.gross, david.brown, georgi.djakov, dianders, swboyd,
	bjorn.andersson
In-Reply-To: <1548138816-1149-1-git-send-email-alokc@codeaurora.org>

Get the interconnect paths for I2C based Serial Engine device
and vote accordingly based on maximum supported I2C frequency.

Signed-off-by: Alok Chauhan <alokc@codeaurora.org>
---
 drivers/i2c/busses/i2c-qcom-geni.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/i2c/busses/i2c-qcom-geni.c b/drivers/i2c/busses/i2c-qcom-geni.c
index db075bc..e8fe63a 100644
--- a/drivers/i2c/busses/i2c-qcom-geni.c
+++ b/drivers/i2c/busses/i2c-qcom-geni.c
@@ -14,6 +14,7 @@
 #include <linux/pm_runtime.h>
 #include <linux/qcom-geni-se.h>
 #include <linux/spinlock.h>
+#include <linux/interconnect.h>
 
 #define SE_I2C_TX_TRANS_LEN		0x26c
 #define SE_I2C_RX_TRANS_LEN		0x270
@@ -508,6 +509,15 @@ static int geni_i2c_probe(struct platform_device *pdev)
 		return ret;
 	}
 
+	/* Set the bus quota to a reasonable value */
+	gi2c->se.avg_bw = Bps_to_icc(1000);
+	gi2c->se.peak_bw = Bps_to_icc(76800000);
+	ret = geni_interconnect_init(&gi2c->se);
+	if (ret) {
+		dev_err(&pdev->dev, "interconnect_init failed %d\n", ret);
+		return ret;
+	}
+
 	ret = device_property_read_u32(&pdev->dev, "clock-frequency",
 							&gi2c->clk_freq_out);
 	if (ret) {
@@ -611,6 +621,8 @@ static int __maybe_unused geni_i2c_runtime_suspend(struct device *dev)
 		gi2c->suspended = 1;
 	}
 
+	geni_icc_update_bw(&gi2c->se, false);
+
 	return 0;
 }
 
@@ -619,6 +631,7 @@ static int __maybe_unused geni_i2c_runtime_resume(struct device *dev)
 	int ret;
 	struct geni_i2c_dev *gi2c = dev_get_drvdata(dev);
 
+	geni_icc_update_bw(&gi2c->se, true);
 	ret = geni_se_resources_on(&gi2c->se);
 	if (ret)
 		return ret;
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply related

* [PATCH 2/6] soc: qcom: Add wrapper to support for Interconnect path
From: Alok Chauhan @ 2019-01-22  6:33 UTC (permalink / raw)
  To: linux-arm-msm, devicetree, linux-kernel, linux-i2c, linux-spi,
	linux-serial, Andy Gross, David Brown, Stephen Boyd,
	Douglas Anderson, Bjorn Andersson, Karthikeyan Ramasubramanian,
	Alok Chauhan
  Cc: georgi.djakov
In-Reply-To: <1548138816-1149-1-git-send-email-alokc@codeaurora.org>

Add the wrapper to support for interconnect path voting
from GENI QUP. This wrapper provides the functionalities
to individual Serial Engine device to get the interconnect
path and to vote for bandwidth based on their need.

This wrapper maintains bandwidth votes from each Serial Engine
and send the aggregated vote from GENI QUP to interconnect
framework.

Signed-off-by: Alok Chauhan <alokc@codeaurora.org>
---
 drivers/soc/qcom/qcom-geni-se.c | 129 ++++++++++++++++++++++++++++++++++++++++
 include/linux/qcom-geni-se.h    |  11 ++++
 2 files changed, 140 insertions(+)

diff --git a/drivers/soc/qcom/qcom-geni-se.c b/drivers/soc/qcom/qcom-geni-se.c
index 6b8ef01..1d8dcb1 100644
--- a/drivers/soc/qcom/qcom-geni-se.c
+++ b/drivers/soc/qcom/qcom-geni-se.c
@@ -11,6 +11,7 @@
 #include <linux/pinctrl/consumer.h>
 #include <linux/platform_device.h>
 #include <linux/qcom-geni-se.h>
+#include <linux/interconnect.h>
 
 /**
  * DOC: Overview
@@ -84,11 +85,21 @@
  * @dev:		Device pointer of the QUP wrapper core
  * @base:		Base address of this instance of QUP wrapper core
  * @ahb_clks:		Handle to the primary & secondary AHB clocks
+ * @icc_path:		Array of interconnect path handles
+ * @geni_wrapper_lock:	Lock to protect the bus bandwidth request
+ * @cur_avg_bw:		Current Bus Average BW request value from GENI QUP
+ * @cur_peak_bw:	Current Bus Peak BW request value from GENI QUP
+ * @peak_bw_list_head:	Sorted resource list based on peak bus BW
  */
 struct geni_wrapper {
 	struct device *dev;
 	void __iomem *base;
 	struct clk_bulk_data ahb_clks[NUM_AHB_CLKS];
+	struct icc_path *icc_path[2];
+	struct mutex geni_wrapper_lock;
+	u32 cur_avg_bw;
+	u32 cur_peak_bw;
+	struct list_head peak_bw_list_head;
 };
 
 #define QUP_HW_VER_REG			0x4
@@ -440,6 +451,71 @@ static void geni_se_clks_off(struct geni_se *se)
 }
 
 /**
+ * geni_icc_update_bw() - Request to update bw vote on an interconnect path
+ * @se:			Pointer to the concerned serial engine.
+ * @update:		Flag to update bw vote.
+ *
+ * This function is used to request set bw vote on interconnect path handle.
+ */
+void geni_icc_update_bw(struct geni_se *se, bool update)
+{
+	struct geni_se *temp = NULL;
+	struct list_head *ins_list_head;
+	struct geni_wrapper *wrapper;
+
+	mutex_lock(&se->wrapper->geni_wrapper_lock);
+
+	wrapper = se->wrapper;
+
+	if (update) {
+		wrapper->cur_avg_bw += se->avg_bw;
+		ins_list_head = &wrapper->peak_bw_list_head;
+		list_for_each_entry(temp, &wrapper->peak_bw_list_head,
+				peak_bw_list) {
+			if (temp->peak_bw < se->peak_bw)
+				break;
+			ins_list_head = &temp->peak_bw_list;
+		}
+
+		list_add(&se->peak_bw_list, ins_list_head);
+
+		if (ins_list_head == &wrapper->peak_bw_list_head)
+			wrapper->cur_peak_bw = se->peak_bw;
+	} else {
+		if (unlikely(list_empty(&se->peak_bw_list))) {
+			mutex_unlock(&wrapper->geni_wrapper_lock);
+			return;
+		}
+
+		wrapper->cur_avg_bw -= se->avg_bw;
+
+		list_del_init(&se->peak_bw_list);
+		temp = list_first_entry_or_null(&wrapper->peak_bw_list_head,
+					struct geni_se, peak_bw_list);
+		if (temp && temp->peak_bw != wrapper->cur_peak_bw)
+			wrapper->cur_peak_bw = temp->peak_bw;
+		else if (!temp && wrapper->cur_peak_bw)
+			wrapper->cur_peak_bw = 0;
+	}
+
+	/*
+	 * This bw vote is to enable internal QUP core clock as well as to
+	 * enable path towards memory.
+	 */
+	icc_set_bw(wrapper->icc_path[0], wrapper->cur_avg_bw,
+		wrapper->cur_peak_bw);
+
+	/*
+	 * This is just register configuration path so doesn't need avg bw.
+	 * Set only peak bw to enable this path.
+	 */
+	icc_set_bw(wrapper->icc_path[1], 0, wrapper->cur_peak_bw);
+
+	mutex_unlock(&wrapper->geni_wrapper_lock);
+}
+EXPORT_SYMBOL(geni_icc_update_bw);
+
+/**
  * geni_se_resources_off() - Turn off resources associated with the serial
  *                           engine
  * @se:	Pointer to the concerned serial engine.
@@ -707,6 +783,47 @@ void geni_se_rx_dma_unprep(struct geni_se *se, dma_addr_t iova, size_t len)
 }
 EXPORT_SYMBOL(geni_se_rx_dma_unprep);
 
+/**
+ * geni_interconnect_init() - Request to get interconnect path handle
+ * @se:			Pointer to the concerned serial engine.
+ *
+ * This function is used to get interconnect path handle.
+ */
+int geni_interconnect_init(struct geni_se *se)
+{
+	struct geni_wrapper *wrapper_rsc;
+
+	if (unlikely(!se || !se->wrapper))
+		return -EINVAL;
+
+	wrapper_rsc = se->wrapper;
+
+	if ((IS_ERR_OR_NULL(wrapper_rsc->icc_path[0]) ||
+		IS_ERR_OR_NULL(wrapper_rsc->icc_path[1]))) {
+
+		wrapper_rsc->icc_path[0] = of_icc_get(wrapper_rsc->dev,
+						"qup-memory");
+		if (IS_ERR(wrapper_rsc->icc_path[0]))
+			return PTR_ERR(wrapper_rsc->icc_path[0]);
+
+		wrapper_rsc->icc_path[1] = of_icc_get(wrapper_rsc->dev,
+						"qup-config");
+		if (IS_ERR(wrapper_rsc->icc_path[1])) {
+			icc_put(wrapper_rsc->icc_path[0]);
+			wrapper_rsc->icc_path[0] = NULL;
+			return PTR_ERR(wrapper_rsc->icc_path[1]);
+		}
+
+		INIT_LIST_HEAD(&wrapper_rsc->peak_bw_list_head);
+		mutex_init(&wrapper_rsc->geni_wrapper_lock);
+	}
+
+	INIT_LIST_HEAD(&se->peak_bw_list);
+
+	return 0;
+}
+EXPORT_SYMBOL(geni_interconnect_init);
+
 static int geni_se_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -737,6 +854,17 @@ static int geni_se_probe(struct platform_device *pdev)
 	return devm_of_platform_populate(dev);
 }
 
+static int geni_se_remove(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct geni_wrapper *wrapper = dev_get_drvdata(dev);
+
+	icc_put(wrapper->icc_path[0]);
+	icc_put(wrapper->icc_path[1]);
+
+	return 0;
+}
+
 static const struct of_device_id geni_se_dt_match[] = {
 	{ .compatible = "qcom,geni-se-qup", },
 	{}
@@ -749,6 +877,7 @@ static int geni_se_probe(struct platform_device *pdev)
 		.of_match_table = geni_se_dt_match,
 	},
 	.probe = geni_se_probe,
+	.remove = geni_se_remove,
 };
 module_platform_driver(geni_se_driver);
 
diff --git a/include/linux/qcom-geni-se.h b/include/linux/qcom-geni-se.h
index 3bcd67f..9bf03e9 100644
--- a/include/linux/qcom-geni-se.h
+++ b/include/linux/qcom-geni-se.h
@@ -33,6 +33,10 @@ enum geni_se_protocol_type {
  * @clk:		Handle to the core serial engine clock
  * @num_clk_levels:	Number of valid clock levels in clk_perf_tbl
  * @clk_perf_tbl:	Table of clock frequency input to serial engine clock
+ * @avg_bw:		Average bus bandwidth value for Serial Engine device
+ * @peak_bw:		Peak bus bandwidth value for Serial Engine device
+ * @peak_bw_list:	List Head of peak bus banwidth list for Serial Engine
+ *			device
  */
 struct geni_se {
 	void __iomem *base;
@@ -41,6 +45,9 @@ struct geni_se {
 	struct clk *clk;
 	unsigned int num_clk_levels;
 	unsigned long *clk_perf_tbl;
+	u32 avg_bw;
+	u32 peak_bw;
+	struct list_head peak_bw_list;
 };
 
 /* Common SE registers */
@@ -416,5 +423,9 @@ int geni_se_rx_dma_prep(struct geni_se *se, void *buf, size_t len,
 void geni_se_tx_dma_unprep(struct geni_se *se, dma_addr_t iova, size_t len);
 
 void geni_se_rx_dma_unprep(struct geni_se *se, dma_addr_t iova, size_t len);
+
+int geni_interconnect_init(struct geni_se *se);
+
+void geni_icc_update_bw(struct geni_se *se, bool update);
 #endif
 #endif
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply related

* [PATCH 1/6] dt-bindings: soc: qcom: Add interconnect binding for GENI QUP
From: Alok Chauhan @ 2019-01-22  6:33 UTC (permalink / raw)
  To: linux-arm-msm, devicetree, linux-kernel, linux-i2c, linux-spi,
	linux-serial, Andy Gross, David Brown, Rob Herring, Mark Rutland
  Cc: georgi.djakov, dianders, swboyd, bjorn.andersson, Alok Chauhan
In-Reply-To: <1548138816-1149-1-git-send-email-alokc@codeaurora.org>

Add documentation for the interconnect and interconnect-names bindings
for the GENI QUP as detailed by bindings/interconnect/interconnect.txt.

Signed-off-by: Alok Chauhan <alokc@codeaurora.org>
---
 Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt b/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt
index dab7ca9..44d7e02 100644
--- a/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt
+++ b/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt
@@ -17,6 +17,12 @@ Required properties if child node exists:
 - #address-cells: 	Must be <1> for Serial Engine Address
 - #size-cells: 		Must be <1> for Serial Engine Address Size
 - ranges: 		Must be present
+- interconnects:	phandle to a interconnect provider. Please refer
+			../interconnect/interconnect.txt for details.
+			Must be 2 paths corresponding to 2 AXI ports.
+- interconnect-names:	Port names to differentiate between the
+			2 interconnect paths defined with interconnect
+			specifier.
 
 Properties for children:
 
@@ -67,6 +73,10 @@ Example:
 		#size-cells = <1>;
 		ranges;
 
+		interconnects = <&qnoc 11 &qnoc 512>,
+				<&qnoc 0 &qnoc 543>;
+		interconnect-names = "qup-memory", "qup-config";
+
 		i2c0: i2c@a94000 {
 			compatible = "qcom,geni-i2c";
 			reg = <0xa94000 0x4000>;
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply related

* [PATCH 0/6] Add interconnect support for GENI QUPs
From: Alok Chauhan @ 2019-01-22  6:33 UTC (permalink / raw)
  To: linux-arm-msm, devicetree, linux-kernel, linux-i2c, linux-spi,
	linux-serial
  Cc: andy.gross, david.brown, georgi.djakov, dianders, swboyd,
	bjorn.andersson, Alok Chauhan

This patch series contains following:
* Add wrapper framework to support interconnect path from GENI QUPs.
  This wrapper enabled and help individual SEs to put their BW request.
  Adding this wrapper make sense because we don't want individual SEs
  to request to interconnect driver separately and put individual bw
  votes from QUP.

  This wrapper framework does the following:
  - Request for interconnect path handle
  - Maintain record of individual SEs' avg/peak bw.
  - Aggregated avg/peak bw based on how many SE's are active and put
    single bw request from QUP

* Interconnect wrapper API calling from I2C, SPI & Uart driver
* dt binding in sdm845 soc for Interconnect path for GENI QUPs
* dt binding documentation


Alok Chauhan (6):
  dt-bindings: soc: qcom: Add interconnect binding for GENI QUP
  soc: qcom: Add wrapper to support for Interconnect path
  i2c: i2c-qcom-geni: Add interconnect support
  spi: spi-geni-qcom: Add interconnect support
  tty: serial: qcom_geni_serial: Add interconnect support
  arm64: dts: sdm845: Add interconnect for GENI QUP

 .../devicetree/bindings/soc/qcom/qcom,geni-se.txt  |  10 ++
 arch/arm64/boot/dts/qcom/sdm845.dtsi               |  14 +++
 drivers/i2c/busses/i2c-qcom-geni.c                 |  13 +++
 drivers/soc/qcom/qcom-geni-se.c                    | 129 +++++++++++++++++++++
 drivers/spi/spi-geni-qcom.c                        |  20 +++-
 drivers/tty/serial/qcom_geni_serial.c              |  27 ++++-
 include/linux/qcom-geni-se.h                       |  11 ++
 7 files changed, 222 insertions(+), 2 deletions(-)

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply

* Re: [PATCH] tty: ldisc: add sysctl to prevent autoloading of ldiscs
From: Theodore Y. Ts'o @ 2019-01-22  0:01 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: kernel-hardening, linux-kernel, linux-serial, Jiri Slaby
In-Reply-To: <20190121162642.GA2944@kroah.com>

On Mon, Jan 21, 2019 at 05:26:42PM +0100, Greg Kroah-Hartman wrote:
> By default, the kernel will automatically load the module of any line
> dicipline that is asked for.  As this sometimes isn't the safest thing
> to do, provide a sysctl to disable this feature.
> 
> By default, we set this to 'y' as that is the historical way that Linux
> has worked, and we do not want to break working systems.  But in the
> future, perhaps this can default to 'n' to prevent this functionality.
> 
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Reviewed-by: Theodore Ts'o <tytso@mit.edu>

^ permalink raw reply

* Re: [PATCH 2/3] dt-bindings: serial: omap_serial: add clocks entry
From: Rob Herring @ 2019-01-21 21:08 UTC (permalink / raw)
  Cc: Greg Kroah-Hartman, Rob Herring, Jiri Slaby, Vignesh R,
	Tony Lindgren, Lokesh Vutla, linux-serial, linux-omap, devicetree,
	linux-kernel
In-Reply-To: <20190109091206.25759-3-vigneshr@ti.com>

On Wed, 9 Jan 2019 14:42:05 +0530, Vignesh R wrote:
> Document clocks property used to pass phandle to functional clk.
> 
> Signed-off-by: Vignesh R <vigneshr@ti.com>
> ---
>  Documentation/devicetree/bindings/serial/omap_serial.txt | 2 ++
>  1 file changed, 2 insertions(+)
> 

Reviewed-by: Rob Herring <robh@kernel.org>

^ permalink raw reply

* Re: [PATCH 0/3] serdev support for n_gsm
From: Tony Lindgren @ 2019-01-21 17:01 UTC (permalink / raw)
  To: Johan Hovold
  Cc: Greg Kroah-Hartman, linux-kernel, Alan Cox, Jiri Slaby,
	Pavel Machek, Peter Hurley, Rob Herring, Sebastian Reichel,
	linux-serial, Marcel Holtmann
In-Reply-To: <20190121105735.GI3691@localhost>

Hi,

* Johan Hovold <johan@kernel.org> [190121 10:57]:
> Adding Marcel on CC.
> 
> On Fri, Jan 18, 2019 at 12:59:58PM +0100, Greg Kroah-Hartman wrote:
> > On Sun, Jan 13, 2019 at 05:25:25PM -0800, Tony Lindgren wrote:
> > > Hi all,
> > > 
> > > Here's a series of patches to add initial serdev support to n_gsm
> > > TS 27.010 line discipline.
> > > 
> > > This allows handling vendor specific protocols on top of TS 27.010 and
> > > allows creating simple serdev drivers where it makes sense. So far I've
> > > tested it with droid 4 for it's modem to provide char devices for AT
> > > ports, modem PM support, and serdev drivers for GNSS and Alsa ASoC.
> > > 
> > > I'll be posting the related MFD, GNSS and Alsa ASoC drivers separately.
> > > For reference, the MFD driver is at [0], the GNSS driver at [1], and
> > > the Alsa ASoC driver at [2] below.
> > 
> > I have applied the first two patches to my tree, as those are nice
> > cleanups.
> > 
> > The last one I want some feedback from the serdev developers to verify
> > all is set up properly, and Johan, to see if this ends up conflicting
> > with the gnss code, as that would not be good.
> 
> I think we need to have a discussion about how to model modems generally
> before getting into implementation details.
> 
> Modems are currently managed by user space (e.g. through ofono) and
> I'm not sure that moving only parts of its responsibilities into the
> kernel is necessarily the right thing to do. You still need coordination
> between the various components for things like power management.

At least now we do have the option of doing kernel drivers or user
space apps whichever way we want to :)

And we can now do the user space apps without having to implement
any of the Motorola custom packet numbering layer on top of TS 27.010
for each app.

For some user space examples, I have posted scripts to send and receive
SMS at [3], and Pavel has ofono patches [4] below. Seems like we can
also add support to ModemManager along the similar lines. And for the
serdev drivers, those implement standard Linux interfaces for apps
to use.

For PM, about a year ago I tried making things work with a user space
solution and it sucked big time[5]. The power management makes sense
to do in the kernel driver at least in this case as there are shared
GPIO pins between the USB PHY and TS 27.010 UART. The shared GPIOs
are handled by the phy-mapphone-mdm6600 driver.

With the serdev n_gsm MFD driver, the only thing that needs to be done
to idle the modem is to enable autosuspend for the OHCI interface. So
no spefific coordination between various components is needed for PM
beyond that. Things idle just fine using PM runtime.

> TS 27.010 may make it seem like we can move everything into the kernel,
> but Tony's to-be-posted Motorola MFD driver is still exposing character
> devices for most of the muxed ports. If I understand things correctly,
> there also still needs to be some coordination with USB over which some
> channels are handled (e.g. IP over USB, gnss over muxed UART).

Hmm yes now we can do either user space daemons or kernel serdev
drivers.

For USB, the modem data connection already works with USB OHCI over
QMI. So it's is already handled and separated out of this. The USB
PHY and TS 27.010 UART have shared GPIO pins handled by the USB PHY
driver. The USB PHY is integrated into the modem with the shared
GPIO pins controlling the PHY and the TS 27.010 UART PM..
But it's working for PM and like I mentioned modem PM works as long
as OHCI is set to autosuspend. Well the modem also wants to see
TS 27.010 connected before idling.

> Instead of adding these extra layers, only to export most ports to user
> space again, it may be better to hook into the various kernel subsystems
> through dedicated user-space-implementation interfaces such as the
> suggested ugnss interface, which means that user space feeds gnss data
> into the kernel which in turn makes it available through a standard
> interface.

Sure that's doable. But notice that we actually need to kick the
serdev GNSS interface to get more data. It's not a passive GNSS
data feed in this case. So it's not going to be just a case of
cat /dev/motmdm4 > /dev/ugnss. Without the serdev GNSS driver,
it would be some-custom-app -i /dev/motmdm4 -o /dev/ugnss.

And without the n_gsm serdev support, it's a mess of some app
similar to [5] initializing n_gsm, trying to deal with the USB
PHY PM, dealing with Motorola custom packet numbering, kicking
GNSS device, feeding data to /dev/ugnss. Hmm I think I've already
been there just to be able to type AT commands to the modem and
it did not work :)

Anyways, for the serdev kernel drivers, the criteria I've tried
to follow is: "Can this serdev device driver make user space
apps use standard Linux interfaces for the hardware?"

So for the serdev Alsa ASoC driver, user space can use the standard
Alsa interface for setting voice call volume. And for the serdev
GNSS driver, user space can use /dev/gnss0.

I don't think it makes sense to do the GNSS driver in user space
in this case because of the kicking needed. But at least it
certainly is doable with /dev/ugnss.

> This would also allow things to work for other transports such as USB
> CDC for which the mux protocol isn't used.

Yeah I agree it would be nice to have GNSS framework have ugnss
device similar to drivers/input/misc/uinput.c. Anyways, having
a serdev GNSS driver in this case does not prevent a USB driver
from feeding data to the /dev/ugnss interface. In this case
doing GNSS driver over UART is preferred for smaller power
consumption.

Regards,

Tony


[0] https://git.kernel.org/pub/scm/linux/kernel/git/tmlind/linux-omap.git/tree/drivers/mfd/motorola-mdm.c?h=droid4-pending-mdm-v4.20
[1] https://git.kernel.org/pub/scm/linux/kernel/git/tmlind/linux-omap.git/tree/drivers/gnss/motmdm.c?h=droid4-pending-mdm-v4.20
[2] https://git.kernel.org/pub/scm/linux/kernel/git/tmlind/linux-omap.git/tree/sound/soc/codecs/motmdm.c?h=droid4-pending-mdm-v4.20
[3] https://github.com/tmlind/droid4-sms-tools
[4] https://github.com/pavelmachek/ofono
[5] https://github.com/tmlind/droid4-ngsm

^ permalink raw reply

* [PATCH] tty: ldisc: add sysctl to prevent autoloading of ldiscs
From: Greg Kroah-Hartman @ 2019-01-21 16:26 UTC (permalink / raw)
  To: kernel-hardening, linux-kernel, linux-serial; +Cc: Jiri Slaby

By default, the kernel will automatically load the module of any line
dicipline that is asked for.  As this sometimes isn't the safest thing
to do, provide a sysctl to disable this feature.

By default, we set this to 'y' as that is the historical way that Linux
has worked, and we do not want to break working systems.  But in the
future, perhaps this can default to 'n' to prevent this functionality.

Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---

Every once in a while people are really surprised that any line
discipline can be loaded by any user.  Despite the ability to just
disable the modules from the modules.conf file, sometimes it is good to
provide protection from within the kernel instead of relying on a module
configuration file.

So that's why I wrote this patch up.  Anyone have any objections to it.
It keeps the functionality that we have today by default, but allows
admins to disable the auto-loading if they want to at runtime.



 drivers/tty/Kconfig     | 24 +++++++++++++++++++++
 drivers/tty/tty_io.c    |  3 +++
 drivers/tty/tty_ldisc.c | 47 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 74 insertions(+)

diff --git a/drivers/tty/Kconfig b/drivers/tty/Kconfig
index 0840d27381ea..e0a04bfc873e 100644
--- a/drivers/tty/Kconfig
+++ b/drivers/tty/Kconfig
@@ -441,4 +441,28 @@ config VCC
 	depends on SUN_LDOMS
 	help
 	  Support for Sun logical domain consoles.
+
+config LDISC_AUTOLOAD
+	bool "Automatically load TTY Line Disciplines"
+	default y
+	help
+	  Historically the kernel has always automatically loaded any
+	  line discipline that is in a kernel module when a user asks
+	  for it to be loaded with the TIOCSETD ioctl, or through other
+	  means.  This is not always the best thing to do on systems
+	  where you know you will not be using some of the more
+	  "ancient" line disciplines, so prevent the kernel from doing
+	  this unless the request is coming from a process with the
+	  CAP_SYS_MODULE permissions.
+
+	  Say 'Y' here if you trust your userspace users to do the right
+	  thing, or if you have only provided the line disciplines that
+	  you know you will be using, or if you wish to continue to use
+	  the traditional method of on-demand loading of these modules
+	  by any user.
+
+	  This functionality can be changed at runtime with the
+	  dev.tty.ldisc_autoload sysctl, this configuration option will
+	  only set the default value of this functionality.
+
 endif # TTY
diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index 21ffcce16927..5fa250157025 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -513,6 +513,8 @@ static const struct file_operations hung_up_tty_fops = {
 static DEFINE_SPINLOCK(redirect_lock);
 static struct file *redirect;
 
+extern void tty_sysctl_init(void);
+
 /**
  *	tty_wakeup	-	request more data
  *	@tty: terminal
@@ -3483,6 +3485,7 @@ void console_sysfs_notify(void)
  */
 int __init tty_init(void)
 {
+	tty_sysctl_init();
 	cdev_init(&tty_cdev, &tty_fops);
 	if (cdev_add(&tty_cdev, MKDEV(TTYAUX_MAJOR, 0), 1) ||
 	    register_chrdev_region(MKDEV(TTYAUX_MAJOR, 0), 1, "/dev/tty") < 0)
diff --git a/drivers/tty/tty_ldisc.c b/drivers/tty/tty_ldisc.c
index 45eda69b150c..e38f104db174 100644
--- a/drivers/tty/tty_ldisc.c
+++ b/drivers/tty/tty_ldisc.c
@@ -156,6 +156,13 @@ static void put_ldops(struct tty_ldisc_ops *ldops)
  *		takes tty_ldiscs_lock to guard against ldisc races
  */
 
+#if defined(CONFIG_LDISC_AUTOLOAD)
+	#define INITIAL_AUTOLOAD_STATE	1
+#else
+	#define INITIAL_AUTOLOAD_STATE	0
+#endif
+static int tty_ldisc_autoload = INITIAL_AUTOLOAD_STATE;
+
 static struct tty_ldisc *tty_ldisc_get(struct tty_struct *tty, int disc)
 {
 	struct tty_ldisc *ld;
@@ -170,6 +177,8 @@ static struct tty_ldisc *tty_ldisc_get(struct tty_struct *tty, int disc)
 	 */
 	ldops = get_ldops(disc);
 	if (IS_ERR(ldops)) {
+		if (!capable(CAP_SYS_MODULE) && !tty_ldisc_autoload)
+			return ERR_PTR(-EPERM);
 		request_module("tty-ldisc-%d", disc);
 		ldops = get_ldops(disc);
 		if (IS_ERR(ldops))
@@ -845,3 +854,41 @@ void tty_ldisc_deinit(struct tty_struct *tty)
 		tty_ldisc_put(tty->ldisc);
 	tty->ldisc = NULL;
 }
+
+static int zero;
+static int one = 1;
+static struct ctl_table tty_table[] = {
+	{
+		.procname	= "ldisc_autoload",
+		.data		= &tty_ldisc_autoload,
+		.maxlen		= sizeof(tty_ldisc_autoload),
+		.mode		= 0644,
+		.proc_handler	= proc_dointvec,
+		.extra1		= &zero,
+		.extra2		= &one,
+	},
+	{ }
+};
+
+static struct ctl_table tty_dir_table[] = {
+	{
+		.procname	= "tty",
+		.mode		= 0555,
+		.child		= tty_table,
+	},
+	{ }
+};
+
+static struct ctl_table tty_root_table[] = {
+	{
+		.procname	= "dev",
+		.mode		= 0555,
+		.child		= tty_dir_table,
+	},
+	{ }
+};
+
+void tty_sysctl_init(void)
+{
+	register_sysctl_table(tty_root_table);
+}
-- 
2.20.1

^ permalink raw reply related

* Re: [BUG] tiocsti() NULL dereference if ld->ops->receive_buf==NULL
From: Greg Kroah-Hartman @ 2019-01-21 16:14 UTC (permalink / raw)
  To: Jann Horn; +Cc: Jiri Slaby, kernel list, linux-serial
In-Reply-To: <CAG48ez0=Mcgk15EWBfVw-X7gB2hMJFULRA_w429-uyPZotdqNA@mail.gmail.com>

On Mon, Jan 21, 2019 at 04:38:33PM +0100, Jann Horn wrote:
> On Sun, Jan 20, 2019 at 10:52 AM Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> >
> > On Sat, Jan 19, 2019 at 10:11:08AM +0100, Greg Kroah-Hartman wrote:
> > > On Fri, Jan 18, 2019 at 08:09:07PM +0100, Jann Horn wrote:
> > > > Hi!
> > > >
> > > > When a line discipline doesn't have a ->receive_buf handler, tiocsti()
> > > > attempts to call a NULL pointer. Both tty_n_tracesink and
> > > > spk_ttyio_ldisc_ops don't have such a handler.
> > > >
> > > > To reproduce, build a kernel with CONFIG_SPEAKUP=y and
> > > > CONFIG_SPEAKUP_SYNTH_SOFT=y, set speakup.synth=soft in the kernel
> > > > command line, and run the following code as root:
> > >
> > > <snip>
> > >
> > > Ugh, thanks for finding this.  I'll look at it later this afternoon...
> >
> > It looks to be a simple change.  We can't really "fail" this ioctl if
> > there's nothing wrong with the structure of the call, so we can just
> > quietly "eat" the character, given that the line discipline doesn't care
> > about it.
> >
> > So, any objections to the patch below?
> 
> No objection from me.
> 
> (spk_ttyio_ldisc_ops has a receive_buf2 handler, but I don't know
> whether that should be invoked here or not.)

No, receive_buf2 can fail, or do a short "receive", which receive_buf()
can't do, and tiocsti can't fail (it's only used to fake input data).

Yeah, the tty layer is strange :(

thanks,

greg k-h

^ permalink raw reply

* Re: [BUG] tiocsti() NULL dereference if ld->ops->receive_buf==NULL
From: Jann Horn @ 2019-01-21 15:38 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Jiri Slaby, kernel list, linux-serial
In-Reply-To: <20190120095205.GB28267@kroah.com>

On Sun, Jan 20, 2019 at 10:52 AM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Sat, Jan 19, 2019 at 10:11:08AM +0100, Greg Kroah-Hartman wrote:
> > On Fri, Jan 18, 2019 at 08:09:07PM +0100, Jann Horn wrote:
> > > Hi!
> > >
> > > When a line discipline doesn't have a ->receive_buf handler, tiocsti()
> > > attempts to call a NULL pointer. Both tty_n_tracesink and
> > > spk_ttyio_ldisc_ops don't have such a handler.
> > >
> > > To reproduce, build a kernel with CONFIG_SPEAKUP=y and
> > > CONFIG_SPEAKUP_SYNTH_SOFT=y, set speakup.synth=soft in the kernel
> > > command line, and run the following code as root:
> >
> > <snip>
> >
> > Ugh, thanks for finding this.  I'll look at it later this afternoon...
>
> It looks to be a simple change.  We can't really "fail" this ioctl if
> there's nothing wrong with the structure of the call, so we can just
> quietly "eat" the character, given that the line discipline doesn't care
> about it.
>
> So, any objections to the patch below?

No objection from me.

(spk_ttyio_ldisc_ops has a receive_buf2 handler, but I don't know
whether that should be invoked here or not.)

> -----------------
>
> Subject: [PATCH] tty: Handle problem if line discipline does not have receive_buf
>
> Some tty line disciplines do not have a receive buf callback, so
> properly check for that before calling it.  If they do not have this
> callback, just eat the character quietly, as we can't fail this call.
>
> Reported-by: Jann Horn <jannh@google.com>
> Cc: stable <stable@vger.kernel.org>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
>  drivers/tty/tty_io.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
> index 23c6fd238422..21ffcce16927 100644
> --- a/drivers/tty/tty_io.c
> +++ b/drivers/tty/tty_io.c
> @@ -2189,7 +2189,8 @@ static int tiocsti(struct tty_struct *tty, char __user *p)
>         ld = tty_ldisc_ref_wait(tty);
>         if (!ld)
>                 return -EIO;
> -       ld->ops->receive_buf(tty, &ch, &mbz, 1);
> +       if (ld->ops->receive_buf)
> +               ld->ops->receive_buf(tty, &ch, &mbz, 1);
>         tty_ldisc_deref(ld);
>         return 0;
>  }
> --
> 2.20.1
>

^ permalink raw reply

* Re: [PATCH] tty: serial: qcom_geni_serial: Allow mctrl when flow control is disabled
From: msavaliy @ 2019-01-21 14:35 UTC (permalink / raw)
  To: Matthias Kaehlcke
  Cc: Greg Kroah-Hartman, Jiri Slaby, linux-serial, linux-kernel,
	Johan Hovold, Balakrishna Godavarthi, linux-kernel-owner
In-Reply-To: <20190119002305.16639-1-mka@chromium.org>

On 2019-01-19 05:53, Matthias Kaehlcke wrote:
> The geni set/get_mctrl() functions currently do nothing unless
> hardware flow control is enabled. Remove this arbitrary limitation.
> 
> Suggested-by: Johan Hovold <johan@kernel.org>
> Fixes: 8a8a66a1a18a ("tty: serial: qcom_geni_serial: Add support for
> flow control")
> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> ---
>  drivers/tty/serial/qcom_geni_serial.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/tty/serial/qcom_geni_serial.c
> b/drivers/tty/serial/qcom_geni_serial.c
> index a72d6d9fb9834..38016609c7fa9 100644
> --- a/drivers/tty/serial/qcom_geni_serial.c
> +++ b/drivers/tty/serial/qcom_geni_serial.c
> @@ -225,7 +225,7 @@ static unsigned int
> qcom_geni_serial_get_mctrl(struct uart_port *uport)
>  	unsigned int mctrl = TIOCM_DSR | TIOCM_CAR;
>  	u32 geni_ios;
> 
> -	if (uart_console(uport) || !uart_cts_enabled(uport)) {
> +	if (uart_console(uport)) {
>  		mctrl |= TIOCM_CTS;
>  	} else {
>  		geni_ios = readl_relaxed(uport->membase + SE_GENI_IOS);
> @@ -241,7 +241,7 @@ static void qcom_geni_serial_set_mctrl(struct
> uart_port *uport,
>  {
>  	u32 uart_manual_rfr = 0;
> 
> -	if (uart_console(uport) || !uart_cts_enabled(uport))
> +	if (uart_console(uport))
>  		return;
> 
>  	if (!(mctrl & TIOCM_RTS))

Though late but wanted to check on why flow control is disabled in a BT 
case ?
If i understand, CRTSCTS at serial core is what makes flow as enabled 
with UPSTAT_CTS_ENABLE set and that in turn returned by 
uart_cts_enabled(uport).
So is there any settings or configuration missing to enable flow control 
?
There could be a case to have 2 wire UART without flow control 
enablement, In that case we may need check for uart_cts_enabled() right 
?
Please add/correct if i missed something.

^ permalink raw reply

* Re: [PATCH v10 3/3] dt-bindings: dma: uart: rename binding
From: Rob Herring @ 2019-01-21 14:18 UTC (permalink / raw)
  To: Long Cheng
  Cc: Vinod Koul, Randy Dunlap, Rob Herring, Mark Rutland, Ryder Lee,
	Sean Wang, Nicolas Boichat, Matthias Brugger, Dan Williams,
	Greg Kroah-Hartman, Jiri Slaby, Sean Wang, dmaengine, devicetree,
	linux-arm-kernel, linux-mediatek, linux-kernel, linux-serial,
	srv_heupstream, Yingjoe Chen, YT Shen, Zhenbao Liu <zhenb>
In-Reply-To: <1547781016-890-4-git-send-email-long.cheng@mediatek.com>

On Fri, 18 Jan 2019 11:10:16 +0800, Long Cheng wrote:
> The filename matches mtk-uart-apdma.c.
> So using "mtk-uart-apdma.txt" should be better.
> 
> Signed-off-by: Long Cheng <long.cheng@mediatek.com>
> ---
>  .../devicetree/bindings/dma/8250_mtk_dma.txt       |   33 --------------------
>  .../devicetree/bindings/dma/mtk-uart-apdma.txt     |   33 ++++++++++++++++++++
>  2 files changed, 33 insertions(+), 33 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/dma/8250_mtk_dma.txt
>  create mode 100644 Documentation/devicetree/bindings/dma/mtk-uart-apdma.txt
> 

Reviewed-by: Rob Herring <robh@kernel.org>

^ permalink raw reply

* [PATCH 5/6] tty: serial: qcom_geni_serial: Add interconnect support
From: Alok Chauhan @ 2019-01-21 11:21 UTC (permalink / raw)
  To: linux-arm-msm, devicetree, Greg Kroah-Hartman, Jiri Slaby,
	linux-serial, linux-kernel
  Cc: andy.gross, david.brown, georgi.djakov, dianders, swboyd,
	bjorn.andersson, Alok Chauhan
In-Reply-To: <1548069703-26595-1-git-send-email-alokc@codeaurora.org>

Get the interconnect paths for Uart based Serial Engine device
and vote accordingly based on maximum supported Uart frequency.

Signed-off-by: Alok Chauhan <alokc@codeaurora.org>
---
 drivers/tty/serial/qcom_geni_serial.c | 27 ++++++++++++++++++++++++++-
 1 file changed, 26 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
index a72d6d9..e2ea499 100644
--- a/drivers/tty/serial/qcom_geni_serial.c
+++ b/drivers/tty/serial/qcom_geni_serial.c
@@ -19,6 +19,7 @@
 #include <linux/slab.h>
 #include <linux/tty.h>
 #include <linux/tty_flip.h>
+#include <linux/interconnect.h>
 
 /* UART specific GENI registers */
 #define SE_UART_LOOPBACK_CFG		0x22c
@@ -1348,6 +1349,22 @@ static int qcom_geni_serial_probe(struct platform_device *pdev)
 		return ret;
 	}
 
+	/* Set the bus quota to a reasonable value */
+	port->se.avg_bw = console ? Bps_to_icc(1000) : Bps_to_icc(2500);
+	port->se.peak_bw = Bps_to_icc(76800000);
+	ret = geni_interconnect_init(&port->se);
+	if (ret) {
+		dev_err(&pdev->dev, "interconnect_init failed %d\n", ret);
+		return ret;
+	}
+
+	/*
+	 * Vote for interconnect path early. This has to move as part of
+	 * Runtime PM APIs when implemented for better control betwen
+	 * console and non-console usecases
+	 */
+	geni_icc_update_bw(&port->se, true);
+
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (!res)
 		return -EINVAL;
@@ -1385,8 +1402,15 @@ static int __maybe_unused qcom_geni_serial_sys_suspend(struct device *dev)
 {
 	struct qcom_geni_serial_port *port = dev_get_drvdata(dev);
 	struct uart_port *uport = &port->uport;
+	int ret;
+
+	ret = uart_suspend_port(uport->private_data, uport);
+	if (ret)
+		return ret;
+
+	geni_icc_update_bw(&port->se, false);
 
-	return uart_suspend_port(uport->private_data, uport);
+	return 0;
 }
 
 static int __maybe_unused qcom_geni_serial_sys_resume(struct device *dev)
@@ -1394,6 +1418,7 @@ static int __maybe_unused qcom_geni_serial_sys_resume(struct device *dev)
 	struct qcom_geni_serial_port *port = dev_get_drvdata(dev);
 	struct uart_port *uport = &port->uport;
 
+	geni_icc_update_bw(&port->se, true);
 	return uart_resume_port(uport->private_data, uport);
 }
 
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply related

* Re: [PATCH 0/3] serdev support for n_gsm
From: Johan Hovold @ 2019-01-21 10:57 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Tony Lindgren, linux-kernel, Alan Cox, Jiri Slaby, Johan Hovold,
	Pavel Machek, Peter Hurley, Rob Herring, Sebastian Reichel,
	linux-serial, Marcel Holtmann
In-Reply-To: <20190118115958.GA5532@kroah.com>

Adding Marcel on CC.

On Fri, Jan 18, 2019 at 12:59:58PM +0100, Greg Kroah-Hartman wrote:
> On Sun, Jan 13, 2019 at 05:25:25PM -0800, Tony Lindgren wrote:
> > Hi all,
> > 
> > Here's a series of patches to add initial serdev support to n_gsm
> > TS 27.010 line discipline.
> > 
> > This allows handling vendor specific protocols on top of TS 27.010 and
> > allows creating simple serdev drivers where it makes sense. So far I've
> > tested it with droid 4 for it's modem to provide char devices for AT
> > ports, modem PM support, and serdev drivers for GNSS and Alsa ASoC.
> > 
> > I'll be posting the related MFD, GNSS and Alsa ASoC drivers separately.
> > For reference, the MFD driver is at [0], the GNSS driver at [1], and
> > the Alsa ASoC driver at [2] below.
> 
> I have applied the first two patches to my tree, as those are nice
> cleanups.
> 
> The last one I want some feedback from the serdev developers to verify
> all is set up properly, and Johan, to see if this ends up conflicting
> with the gnss code, as that would not be good.

I think we need to have a discussion about how to model modems generally
before getting into implementation details.

Modems are currently managed by user space (e.g. through ofono) and
I'm not sure that moving only parts of its responsibilities into the
kernel is necessarily the right thing to do. You still need coordination
between the various components for things like power management.

TS 27.010 may make it seem like we can move everything into the kernel,
but Tony's to-be-posted Motorola MFD driver is still exposing character
devices for most of the muxed ports. If I understand things correctly,
there also still needs to be some coordination with USB over which some
channels are handled (e.g. IP over USB, gnss over muxed UART).

Instead of adding these extra layers, only to export most ports to user
space again, it may be better to hook into the various kernel subsystems
through dedicated user-space-implementation interfaces such as the
suggested ugnss interface, which means that user space feeds gnss data
into the kernel which in turn makes it available through a standard
interface.

This would also allow things to work for other transports such as USB
CDC for which the mux protocol isn't used.

Johan

^ permalink raw reply

* Re: [PATCH] tty: serial: qcom_geni_serial: Allow mctrl when flow control is disabled
From: Johan Hovold @ 2019-01-21  8:47 UTC (permalink / raw)
  To: Matthias Kaehlcke
  Cc: Greg Kroah-Hartman, Jiri Slaby, linux-serial, linux-kernel,
	Johan Hovold, Balakrishna Godavarthi
In-Reply-To: <20190119002305.16639-1-mka@chromium.org>

On Fri, Jan 18, 2019 at 04:23:05PM -0800, Matthias Kaehlcke wrote:
> The geni set/get_mctrl() functions currently do nothing unless
> hardware flow control is enabled. Remove this arbitrary limitation.
> 
> Suggested-by: Johan Hovold <johan@kernel.org>
> Fixes: 8a8a66a1a18a ("tty: serial: qcom_geni_serial: Add support for flow control")
> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>

Good to hear this was all that was needed. There don't happen to be any
publicly available documentation of these registers?

> ---
>  drivers/tty/serial/qcom_geni_serial.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
> index a72d6d9fb9834..38016609c7fa9 100644
> --- a/drivers/tty/serial/qcom_geni_serial.c
> +++ b/drivers/tty/serial/qcom_geni_serial.c
> @@ -225,7 +225,7 @@ static unsigned int qcom_geni_serial_get_mctrl(struct uart_port *uport)
>  	unsigned int mctrl = TIOCM_DSR | TIOCM_CAR;
>  	u32 geni_ios;
>  
> -	if (uart_console(uport) || !uart_cts_enabled(uport)) {
> +	if (uart_console(uport)) {
>  		mctrl |= TIOCM_CTS;
>  	} else {
>  		geni_ios = readl_relaxed(uport->membase + SE_GENI_IOS);
> @@ -241,7 +241,7 @@ static void qcom_geni_serial_set_mctrl(struct uart_port *uport,
>  {
>  	u32 uart_manual_rfr = 0;
>  
> -	if (uart_console(uport) || !uart_cts_enabled(uport))
> +	if (uart_console(uport))
>  		return;
>  
>  	if (!(mctrl & TIOCM_RTS))

Ignoring mctrl when the port is a console looks broken too by the way.
The driver parses and handles the flow control parameter, but these
conditionals later overrides it.

Could be fixed in the same patch or in a follow-up patch.

Either way, you can add my 

Reviewed-by: Johan Hovold <johan@kernel.org>

Johan

^ permalink raw reply

* Re: [PATCH v9 1/2] dmaengine: 8250_mtk_dma: add MediaTek uart DMA support
From: Long Cheng @ 2019-01-21  8:12 UTC (permalink / raw)
  To: Vinod Koul
  Cc: Randy Dunlap, Rob Herring, Mark Rutland, Ryder Lee, Sean Wang,
	Nicolas Boichat, Matthias Brugger, Dan Williams,
	Greg Kroah-Hartman, Jiri Slaby, Sean Wang, dmaengine, devicetree,
	linux-arm-kernel, linux-mediatek, linux-kernel, linux-serial,
	srv_heupstream, Yingjoe Chen, YT Shen, Zhenbao Liu
In-Reply-To: <20190120052750.GN4635@vkoul-mobl>

On Sun, 2019-01-20 at 10:57 +0530, Vinod Koul wrote:
> On 10-01-19, 18:33, Long Cheng wrote:
> > On Fri, 2019-01-04 at 22:49 +0530, Vinod Koul wrote:
> > > On 02-01-19, 10:12, Long Cheng wrote:
> > > > In DMA engine framework, add 8250 uart dma to support MediaTek uart.
> > > > If MediaTek uart enabled(SERIAL_8250_MT6577), and want to improve
> > > > the performance, can enable the function.
> > > 
> > > Is the DMA controller UART specific, can it work with other controllers
> > > as well, if so you should get rid of uart name in patch
> > 
> > I don't know that it can work or not on other controller. but it's for
> > MediaTek SOC
> 
> What I meant was that if can work with other controllers (users) apart
> from UART, how about say audio, spi etc!!
> 

it's just for UART APDMA. can't work on spi ...

> > 
> > > > +#define MTK_UART_APDMA_CHANNELS		(CONFIG_SERIAL_8250_NR_UARTS * 2)
> > > 
> > > Why are the channels not coming from DT?
> > > 
> > 
> > i will using dma-requests install of it.
> > 
> > > > +
> > > > +#define VFF_EN_B		BIT(0)
> > > > +#define VFF_STOP_B		BIT(0)
> > > > +#define VFF_FLUSH_B		BIT(0)
> > > > +#define VFF_4G_SUPPORT_B	BIT(0)
> > > > +#define VFF_RX_INT_EN0_B	BIT(0)	/*rx valid size >=  vff thre*/
> > > > +#define VFF_RX_INT_EN1_B	BIT(1)
> > > > +#define VFF_TX_INT_EN_B		BIT(0)	/*tx left size >= vff thre*/
> > > 
> > > space around /* space */ also run checkpatch to check for style errors
> > > 
> > 
> > ok.
> > 
> > > > +static void mtk_uart_apdma_start_tx(struct mtk_chan *c)
> > > > +{
> > > > +	unsigned int len, send, left, wpt, d_wpt, tmp;
> > > > +	int ret;
> > > > +
> > > > +	left = mtk_uart_apdma_read(c, VFF_LEFT_SIZE);
> > > > +	if (!left) {
> > > > +		mtk_uart_apdma_write(c, VFF_INT_EN, VFF_TX_INT_EN_B);
> > > > +		return;
> > > > +	}
> > > > +
> > > > +	/* Wait 1sec for flush,  can't sleep*/
> > > > +	ret = readx_poll_timeout(readl, c->base + VFF_FLUSH, tmp,
> > > > +			tmp != VFF_FLUSH_B, 0, 1000000);
> > > > +	if (ret)
> > > > +		dev_warn(c->vc.chan.device->dev, "tx: fail, debug=0x%x\n",
> > > > +			mtk_uart_apdma_read(c, VFF_DEBUG_STATUS));
> > > > +
> > > > +	send = min_t(unsigned int, left, c->desc->avail_len);
> > > > +	wpt = mtk_uart_apdma_read(c, VFF_WPT);
> > > > +	len = mtk_uart_apdma_read(c, VFF_LEN);
> > > > +
> > > > +	d_wpt = wpt + send;
> > > > +	if ((d_wpt & VFF_RING_SIZE) >= len) {
> > > > +		d_wpt = d_wpt - len;
> > > > +		d_wpt = d_wpt ^ VFF_RING_WRAP;
> > > > +	}
> > > > +	mtk_uart_apdma_write(c, VFF_WPT, d_wpt);
> > > > +
> > > > +	c->desc->avail_len -= send;
> > > > +
> > > > +	mtk_uart_apdma_write(c, VFF_INT_EN, VFF_TX_INT_EN_B);
> > > > +	if (mtk_uart_apdma_read(c, VFF_FLUSH) == 0U)
> > > > +		mtk_uart_apdma_write(c, VFF_FLUSH, VFF_FLUSH_B);
> > > > +}
> > > > +
> > > > +static void mtk_uart_apdma_start_rx(struct mtk_chan *c)
> > > > +{
> > > > +	struct mtk_uart_apdma_desc *d = c->desc;
> > > > +	unsigned int len, wg, rg, cnt;
> > > > +
> > > > +	if ((mtk_uart_apdma_read(c, VFF_VALID_SIZE) == 0U) ||
> > > > +		!d || !vchan_next_desc(&c->vc))
> > > > +		return;
> > > > +
> > > > +	len = mtk_uart_apdma_read(c, VFF_LEN);
> > > > +	rg = mtk_uart_apdma_read(c, VFF_RPT);
> > > > +	wg = mtk_uart_apdma_read(c, VFF_WPT);
> > > > +	if ((rg ^ wg) & VFF_RING_WRAP)
> > > > +		cnt = (wg & VFF_RING_SIZE) + len - (rg & VFF_RING_SIZE);
> > > > +	else
> > > > +		cnt = (wg & VFF_RING_SIZE) - (rg & VFF_RING_SIZE);
> > > > +
> > > > +	c->rx_status = cnt;
> > > > +	mtk_uart_apdma_write(c, VFF_RPT, wg);
> > > > +
> > > > +	list_del(&d->vd.node);
> > > > +	vchan_cookie_complete(&d->vd);
> > > > +}
> > > 
> > > this looks odd, why do you have different rx and tx start routines?
> > > 
> > 
> > Would you like explain it in more detail? thanks.
> > In tx function, will wait the last data flush done. and the count the
> > size that send.
> > In Rx function, will count the size that receive.
> > Any way, in rx / tx, need andle WPT or RPT.
> > 
> > > > +static int mtk_uart_apdma_alloc_chan_resources(struct dma_chan *chan)
> > > > +{
> > > > +	struct mtk_uart_apdmadev *mtkd = to_mtk_uart_apdma_dev(chan->device);
> > > > +	struct mtk_chan *c = to_mtk_uart_apdma_chan(chan);
> > > > +	u32 tmp;
> > > > +	int ret;
> > > > +
> > > > +	pm_runtime_get_sync(mtkd->ddev.dev);
> > > > +
> > > > +	mtk_uart_apdma_write(c, VFF_ADDR, 0);
> > > > +	mtk_uart_apdma_write(c, VFF_THRE, 0);
> > > > +	mtk_uart_apdma_write(c, VFF_LEN, 0);
> > > > +	mtk_uart_apdma_write(c, VFF_RST, VFF_WARM_RST_B);
> > > > +
> > > > +	ret = readx_poll_timeout(readl, c->base + VFF_EN, tmp,
> > > > +			tmp == 0, 10, 100);
> > > > +	if (ret) {
> > > > +		dev_err(chan->device->dev, "dma reset: fail, timeout\n");
> > > > +		return ret;
> > > > +	}
> > > 
> > > register read does reset?
> > > 
> > 
> > 'mtk_uart_apdma_write(c, VFF_RST, VFF_WARM_RST_B);' is reset. resd just
> > poll reset done.
> > 
> > > > +
> > > > +	if (!c->requested) {
> > > > +		c->requested = true;
> > > > +		ret = request_irq(mtkd->dma_irq[chan->chan_id],
> > > > +				  mtk_uart_apdma_irq_handler, IRQF_TRIGGER_NONE,
> > > > +				  KBUILD_MODNAME, chan);
> > > 
> > > why is the irq not requested in driver probe?
> > > 
> > 
> > I have explained in below,
> > http://lists.infradead.org/pipermail/linux-mediatek/2018-December/016418.html
> > 
> > > > +static enum dma_status mtk_uart_apdma_tx_status(struct dma_chan *chan,
> > > > +					 dma_cookie_t cookie,
> > > > +					 struct dma_tx_state *txstate)
> > > > +{
> > > > +	struct mtk_chan *c = to_mtk_uart_apdma_chan(chan);
> > > > +	enum dma_status ret;
> > > > +	unsigned long flags;
> > > > +
> > > > +	if (!txstate)
> > > > +		return DMA_ERROR;
> > > > +
> > > > +	ret = dma_cookie_status(chan, cookie, txstate);
> > > > +	spin_lock_irqsave(&c->vc.lock, flags);
> > > > +	if (ret == DMA_IN_PROGRESS) {
> > > > +		c->rx_status = mtk_uart_apdma_read(c, VFF_RPT) & VFF_RING_SIZE;
> > > > +		dma_set_residue(txstate, c->rx_status);
> > > > +	} else if (ret == DMA_COMPLETE && c->cfg.direction == DMA_DEV_TO_MEM) {
> > > 
> > > why set reside when it is complete? also reside can be null, that should
> > > be checked as well
> > > 
> > In different status, set different reside.
> 
> Can you explain that..
> 

RPT is different every time. When RX interrupt coming, update the
rx_status and notify uart that there have data. One transfer is
complete. Uart will call tx_status to get the length at the time of
interruption. Other case, if want to get the tx_status, directly read
register.

> > 
> > > > +static struct dma_async_tx_descriptor *mtk_uart_apdma_prep_slave_sg
> > > > +	(struct dma_chan *chan, struct scatterlist *sgl,
> > > > +	unsigned int sglen, enum dma_transfer_direction dir,
> > > > +	unsigned long tx_flags, void *context)
> > > > +{
> > > > +	struct mtk_chan *c = to_mtk_uart_apdma_chan(chan);
> > > > +	struct mtk_uart_apdma_desc *d;
> > > > +
> > > > +	if ((dir != DMA_DEV_TO_MEM) &&
> > > > +		(dir != DMA_MEM_TO_DEV)) {
> > > > +		dev_err(chan->device->dev, "bad direction\n");
> > > > +		return NULL;
> > > > +	}
> > > 
> > > we have a macro for this
> > 
> > Thanks for your suggestion. i will modify it.
> > > 
> > > > +
> > > > +	/* Now allocate and setup the descriptor */
> > > > +	d = kzalloc(sizeof(*d), GFP_ATOMIC);
> > > > +	if (!d)
> > > > +		return NULL;
> > > > +
> > > > +	/* sglen is 1 */
> > > 
> > > ?

dmaengine_prep_slave_single is called. the parameter 'sglen' is 1.
Add annotation to indicate that the total length is sg_dma_len(sgl), no
more.

> > > 
> > > > +static int mtk_uart_apdma_slave_config(struct dma_chan *chan,
> > > > +				struct dma_slave_config *cfg)
> > > > +{
> > > > +	struct mtk_chan *c = to_mtk_uart_apdma_chan(chan);
> > > > +	struct mtk_uart_apdmadev *mtkd =
> > > > +				to_mtk_uart_apdma_dev(c->vc.chan.device);
> > > > +
> > > > +	c->cfg = *cfg;
> > > > +
> > > > +	if (cfg->direction == DMA_DEV_TO_MEM) {
> > > 
> > > fg->direction is deprecated, in fact I have removed all users recently,
> > > please do not use this
> > 
> > You will remove 'direction' in 'struct dma_slave_config'? if remove, how
> > do confirm direction?
> 
> Yes please use the direction argument in prep_xx calls

I had modified on v10. thanks.

^ permalink raw reply

* Re: [BUG] tiocsti() NULL dereference if ld->ops->receive_buf==NULL
From: Greg Kroah-Hartman @ 2019-01-20  9:52 UTC (permalink / raw)
  To: Jann Horn; +Cc: Jiri Slaby, kernel list, linux-serial
In-Reply-To: <20190119091108.GF10836@kroah.com>

On Sat, Jan 19, 2019 at 10:11:08AM +0100, Greg Kroah-Hartman wrote:
> On Fri, Jan 18, 2019 at 08:09:07PM +0100, Jann Horn wrote:
> > Hi!
> > 
> > When a line discipline doesn't have a ->receive_buf handler, tiocsti()
> > attempts to call a NULL pointer. Both tty_n_tracesink and
> > spk_ttyio_ldisc_ops don't have such a handler.
> > 
> > To reproduce, build a kernel with CONFIG_SPEAKUP=y and
> > CONFIG_SPEAKUP_SYNTH_SOFT=y, set speakup.synth=soft in the kernel
> > command line, and run the following code as root:
> 
> <snip>
> 
> Ugh, thanks for finding this.  I'll look at it later this afternoon...

It looks to be a simple change.  We can't really "fail" this ioctl if
there's nothing wrong with the structure of the call, so we can just
quietly "eat" the character, given that the line discipline doesn't care
about it.

So, any objections to the patch below?

thanks,

greg k-h

-----------------

Subject: [PATCH] tty: Handle problem if line discipline does not have receive_buf

Some tty line disciplines do not have a receive buf callback, so
properly check for that before calling it.  If they do not have this
callback, just eat the character quietly, as we can't fail this call.

Reported-by: Jann Horn <jannh@google.com>
Cc: stable <stable@vger.kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/tty/tty_io.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index 23c6fd238422..21ffcce16927 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -2189,7 +2189,8 @@ static int tiocsti(struct tty_struct *tty, char __user *p)
 	ld = tty_ldisc_ref_wait(tty);
 	if (!ld)
 		return -EIO;
-	ld->ops->receive_buf(tty, &ch, &mbz, 1);
+	if (ld->ops->receive_buf)
+		ld->ops->receive_buf(tty, &ch, &mbz, 1);
 	tty_ldisc_deref(ld);
 	return 0;
 }
-- 
2.20.1

^ permalink raw reply related

* Re: [PATCH v9 1/2] dmaengine: 8250_mtk_dma: add MediaTek uart DMA support
From: Vinod Koul @ 2019-01-20  5:27 UTC (permalink / raw)
  To: Long Cheng
  Cc: Mark Rutland, devicetree, Nicolas Boichat, Zhenbao Liu,
	linux-serial, srv_heupstream, Greg Kroah-Hartman, Randy Dunlap,
	linux-kernel, Rob Herring, Sean Wang, YT Shen, dmaengine,
	Ryder Lee, linux-mediatek, Sean Wang, Jiri Slaby,
	Matthias Brugger, Yingjoe Chen, Dan Williams, linux-arm-kernel
In-Reply-To: <1547116431.3831.43.camel@mhfsdcap03>

On 10-01-19, 18:33, Long Cheng wrote:
> On Fri, 2019-01-04 at 22:49 +0530, Vinod Koul wrote:
> > On 02-01-19, 10:12, Long Cheng wrote:
> > > In DMA engine framework, add 8250 uart dma to support MediaTek uart.
> > > If MediaTek uart enabled(SERIAL_8250_MT6577), and want to improve
> > > the performance, can enable the function.
> > 
> > Is the DMA controller UART specific, can it work with other controllers
> > as well, if so you should get rid of uart name in patch
> 
> I don't know that it can work or not on other controller. but it's for
> MediaTek SOC

What I meant was that if can work with other controllers (users) apart
from UART, how about say audio, spi etc!!

> 
> > > +#define MTK_UART_APDMA_CHANNELS		(CONFIG_SERIAL_8250_NR_UARTS * 2)
> > 
> > Why are the channels not coming from DT?
> > 
> 
> i will using dma-requests install of it.
> 
> > > +
> > > +#define VFF_EN_B		BIT(0)
> > > +#define VFF_STOP_B		BIT(0)
> > > +#define VFF_FLUSH_B		BIT(0)
> > > +#define VFF_4G_SUPPORT_B	BIT(0)
> > > +#define VFF_RX_INT_EN0_B	BIT(0)	/*rx valid size >=  vff thre*/
> > > +#define VFF_RX_INT_EN1_B	BIT(1)
> > > +#define VFF_TX_INT_EN_B		BIT(0)	/*tx left size >= vff thre*/
> > 
> > space around /* space */ also run checkpatch to check for style errors
> > 
> 
> ok.
> 
> > > +static void mtk_uart_apdma_start_tx(struct mtk_chan *c)
> > > +{
> > > +	unsigned int len, send, left, wpt, d_wpt, tmp;
> > > +	int ret;
> > > +
> > > +	left = mtk_uart_apdma_read(c, VFF_LEFT_SIZE);
> > > +	if (!left) {
> > > +		mtk_uart_apdma_write(c, VFF_INT_EN, VFF_TX_INT_EN_B);
> > > +		return;
> > > +	}
> > > +
> > > +	/* Wait 1sec for flush,  can't sleep*/
> > > +	ret = readx_poll_timeout(readl, c->base + VFF_FLUSH, tmp,
> > > +			tmp != VFF_FLUSH_B, 0, 1000000);
> > > +	if (ret)
> > > +		dev_warn(c->vc.chan.device->dev, "tx: fail, debug=0x%x\n",
> > > +			mtk_uart_apdma_read(c, VFF_DEBUG_STATUS));
> > > +
> > > +	send = min_t(unsigned int, left, c->desc->avail_len);
> > > +	wpt = mtk_uart_apdma_read(c, VFF_WPT);
> > > +	len = mtk_uart_apdma_read(c, VFF_LEN);
> > > +
> > > +	d_wpt = wpt + send;
> > > +	if ((d_wpt & VFF_RING_SIZE) >= len) {
> > > +		d_wpt = d_wpt - len;
> > > +		d_wpt = d_wpt ^ VFF_RING_WRAP;
> > > +	}
> > > +	mtk_uart_apdma_write(c, VFF_WPT, d_wpt);
> > > +
> > > +	c->desc->avail_len -= send;
> > > +
> > > +	mtk_uart_apdma_write(c, VFF_INT_EN, VFF_TX_INT_EN_B);
> > > +	if (mtk_uart_apdma_read(c, VFF_FLUSH) == 0U)
> > > +		mtk_uart_apdma_write(c, VFF_FLUSH, VFF_FLUSH_B);
> > > +}
> > > +
> > > +static void mtk_uart_apdma_start_rx(struct mtk_chan *c)
> > > +{
> > > +	struct mtk_uart_apdma_desc *d = c->desc;
> > > +	unsigned int len, wg, rg, cnt;
> > > +
> > > +	if ((mtk_uart_apdma_read(c, VFF_VALID_SIZE) == 0U) ||
> > > +		!d || !vchan_next_desc(&c->vc))
> > > +		return;
> > > +
> > > +	len = mtk_uart_apdma_read(c, VFF_LEN);
> > > +	rg = mtk_uart_apdma_read(c, VFF_RPT);
> > > +	wg = mtk_uart_apdma_read(c, VFF_WPT);
> > > +	if ((rg ^ wg) & VFF_RING_WRAP)
> > > +		cnt = (wg & VFF_RING_SIZE) + len - (rg & VFF_RING_SIZE);
> > > +	else
> > > +		cnt = (wg & VFF_RING_SIZE) - (rg & VFF_RING_SIZE);
> > > +
> > > +	c->rx_status = cnt;
> > > +	mtk_uart_apdma_write(c, VFF_RPT, wg);
> > > +
> > > +	list_del(&d->vd.node);
> > > +	vchan_cookie_complete(&d->vd);
> > > +}
> > 
> > this looks odd, why do you have different rx and tx start routines?
> > 
> 
> Would you like explain it in more detail? thanks.
> In tx function, will wait the last data flush done. and the count the
> size that send.
> In Rx function, will count the size that receive.
> Any way, in rx / tx, need andle WPT or RPT.
> 
> > > +static int mtk_uart_apdma_alloc_chan_resources(struct dma_chan *chan)
> > > +{
> > > +	struct mtk_uart_apdmadev *mtkd = to_mtk_uart_apdma_dev(chan->device);
> > > +	struct mtk_chan *c = to_mtk_uart_apdma_chan(chan);
> > > +	u32 tmp;
> > > +	int ret;
> > > +
> > > +	pm_runtime_get_sync(mtkd->ddev.dev);
> > > +
> > > +	mtk_uart_apdma_write(c, VFF_ADDR, 0);
> > > +	mtk_uart_apdma_write(c, VFF_THRE, 0);
> > > +	mtk_uart_apdma_write(c, VFF_LEN, 0);
> > > +	mtk_uart_apdma_write(c, VFF_RST, VFF_WARM_RST_B);
> > > +
> > > +	ret = readx_poll_timeout(readl, c->base + VFF_EN, tmp,
> > > +			tmp == 0, 10, 100);
> > > +	if (ret) {
> > > +		dev_err(chan->device->dev, "dma reset: fail, timeout\n");
> > > +		return ret;
> > > +	}
> > 
> > register read does reset?
> > 
> 
> 'mtk_uart_apdma_write(c, VFF_RST, VFF_WARM_RST_B);' is reset. resd just
> poll reset done.
> 
> > > +
> > > +	if (!c->requested) {
> > > +		c->requested = true;
> > > +		ret = request_irq(mtkd->dma_irq[chan->chan_id],
> > > +				  mtk_uart_apdma_irq_handler, IRQF_TRIGGER_NONE,
> > > +				  KBUILD_MODNAME, chan);
> > 
> > why is the irq not requested in driver probe?
> > 
> 
> I have explained in below,
> http://lists.infradead.org/pipermail/linux-mediatek/2018-December/016418.html
> 
> > > +static enum dma_status mtk_uart_apdma_tx_status(struct dma_chan *chan,
> > > +					 dma_cookie_t cookie,
> > > +					 struct dma_tx_state *txstate)
> > > +{
> > > +	struct mtk_chan *c = to_mtk_uart_apdma_chan(chan);
> > > +	enum dma_status ret;
> > > +	unsigned long flags;
> > > +
> > > +	if (!txstate)
> > > +		return DMA_ERROR;
> > > +
> > > +	ret = dma_cookie_status(chan, cookie, txstate);
> > > +	spin_lock_irqsave(&c->vc.lock, flags);
> > > +	if (ret == DMA_IN_PROGRESS) {
> > > +		c->rx_status = mtk_uart_apdma_read(c, VFF_RPT) & VFF_RING_SIZE;
> > > +		dma_set_residue(txstate, c->rx_status);
> > > +	} else if (ret == DMA_COMPLETE && c->cfg.direction == DMA_DEV_TO_MEM) {
> > 
> > why set reside when it is complete? also reside can be null, that should
> > be checked as well
> > 
> In different status, set different reside.

Can you explain that..

> 
> > > +static struct dma_async_tx_descriptor *mtk_uart_apdma_prep_slave_sg
> > > +	(struct dma_chan *chan, struct scatterlist *sgl,
> > > +	unsigned int sglen, enum dma_transfer_direction dir,
> > > +	unsigned long tx_flags, void *context)
> > > +{
> > > +	struct mtk_chan *c = to_mtk_uart_apdma_chan(chan);
> > > +	struct mtk_uart_apdma_desc *d;
> > > +
> > > +	if ((dir != DMA_DEV_TO_MEM) &&
> > > +		(dir != DMA_MEM_TO_DEV)) {
> > > +		dev_err(chan->device->dev, "bad direction\n");
> > > +		return NULL;
> > > +	}
> > 
> > we have a macro for this
> 
> Thanks for your suggestion. i will modify it.
> > 
> > > +
> > > +	/* Now allocate and setup the descriptor */
> > > +	d = kzalloc(sizeof(*d), GFP_ATOMIC);
> > > +	if (!d)
> > > +		return NULL;
> > > +
> > > +	/* sglen is 1 */
> > 
> > ?
> > 
> > > +static int mtk_uart_apdma_slave_config(struct dma_chan *chan,
> > > +				struct dma_slave_config *cfg)
> > > +{
> > > +	struct mtk_chan *c = to_mtk_uart_apdma_chan(chan);
> > > +	struct mtk_uart_apdmadev *mtkd =
> > > +				to_mtk_uart_apdma_dev(c->vc.chan.device);
> > > +
> > > +	c->cfg = *cfg;
> > > +
> > > +	if (cfg->direction == DMA_DEV_TO_MEM) {
> > 
> > fg->direction is deprecated, in fact I have removed all users recently,
> > please do not use this
> 
> You will remove 'direction' in 'struct dma_slave_config'? if remove, how
> do confirm direction?

Yes please use the direction argument in prep_xx calls
-- 
~Vinod

^ permalink raw reply

* [PATCH] tty: serial: qcom_geni_serial: Allow mctrl when flow control is disabled
From: Matthias Kaehlcke @ 2019-01-19  0:23 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Jiri Slaby
  Cc: linux-serial, linux-kernel, Johan Hovold, Balakrishna Godavarthi,
	Matthias Kaehlcke

The geni set/get_mctrl() functions currently do nothing unless
hardware flow control is enabled. Remove this arbitrary limitation.

Suggested-by: Johan Hovold <johan@kernel.org>
Fixes: 8a8a66a1a18a ("tty: serial: qcom_geni_serial: Add support for flow control")
Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
---
 drivers/tty/serial/qcom_geni_serial.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
index a72d6d9fb9834..38016609c7fa9 100644
--- a/drivers/tty/serial/qcom_geni_serial.c
+++ b/drivers/tty/serial/qcom_geni_serial.c
@@ -225,7 +225,7 @@ static unsigned int qcom_geni_serial_get_mctrl(struct uart_port *uport)
 	unsigned int mctrl = TIOCM_DSR | TIOCM_CAR;
 	u32 geni_ios;
 
-	if (uart_console(uport) || !uart_cts_enabled(uport)) {
+	if (uart_console(uport)) {
 		mctrl |= TIOCM_CTS;
 	} else {
 		geni_ios = readl_relaxed(uport->membase + SE_GENI_IOS);
@@ -241,7 +241,7 @@ static void qcom_geni_serial_set_mctrl(struct uart_port *uport,
 {
 	u32 uart_manual_rfr = 0;
 
-	if (uart_console(uport) || !uart_cts_enabled(uport))
+	if (uart_console(uport))
 		return;
 
 	if (!(mctrl & TIOCM_RTS))
-- 
2.20.1.321.g9e740568ce-goog

^ permalink raw reply related

* Re: [PATCH] serial: 8250: Fix serial8250 initialization crash
From: Greg KH @ 2019-01-18 12:02 UTC (permalink / raw)
  To: zhe.he
  Cc: jslaby, andriy.shevchenko, bigeasy, Jisheng.Zhang, darwin.dingel,
	linux-serial, linux-kernel
In-Reply-To: <1547715619-181299-1-git-send-email-zhe.he@windriver.com>

On Thu, Jan 17, 2019 at 05:00:19PM +0800, zhe.he@windriver.com wrote:
> From: He Zhe <zhe.he@windriver.com>
> 
> The initialization code of interrupt backoff work might reference NULL
> pointer and cause the following crash, if no port was found.
> 
> [   10.017727] CPU 0 Unable to handle kernel paging request at virtual address 000001b0, epc == 807088e0, ra == 8070863c
> ---- snip ----
> [   11.704470] [<807088e0>] serial8250_register_8250_port+0x318/0x4ac
> [   11.747251] [<80708d74>] serial8250_probe+0x148/0x1c0
> [   11.789301] [<80728450>] platform_drv_probe+0x40/0x94
> [   11.830515] [<807264f8>] really_probe+0xf8/0x318
> [   11.870876] [<80726b7c>] __driver_attach+0x110/0x12c
> [   11.910960] [<80724374>] bus_for_each_dev+0x78/0xcc
> [   11.951134] [<80725958>] bus_add_driver+0x200/0x234
> [   11.989756] [<807273d8>] driver_register+0x84/0x148
> [   12.029832] [<80d72f84>] serial8250_init+0x138/0x198
> [   12.070447] [<80100e6c>] do_one_initcall+0x5c/0x2a0
> [   12.110104] [<80d3a208>] kernel_init_freeable+0x370/0x484
> [   12.150722] [<80a49420>] kernel_init+0x10/0xf8
> [   12.191517] [<8010756c>] ret_from_kernel_thread+0x14/0x1c
> 
> This patch makes sure the initialization code can be reached only if a port
> is found.
> 
> Fixes: commit 6d7f677a2afa ("serial: 8250: Rate limit serial port rx interrupts during input overruns")

No need for the "commit" in this string, this can just be written as:

Fixes: 6d7f677a2afa ("serial: 8250: Rate limit serial port rx interrupts during input overruns")

thanks,

greg k-h

^ permalink raw reply

* Re: [PATCH 0/3] serdev support for n_gsm
From: Greg Kroah-Hartman @ 2019-01-18 11:59 UTC (permalink / raw)
  To: Tony Lindgren
  Cc: linux-kernel, Alan Cox, Jiri Slaby, Johan Hovold, Pavel Machek,
	Peter Hurley, Rob Herring, Sebastian Reichel, linux-serial
In-Reply-To: <20190114012528.2367-1-tony@atomide.com>

On Sun, Jan 13, 2019 at 05:25:25PM -0800, Tony Lindgren wrote:
> Hi all,
> 
> Here's a series of patches to add initial serdev support to n_gsm
> TS 27.010 line discipline.
> 
> This allows handling vendor specific protocols on top of TS 27.010 and
> allows creating simple serdev drivers where it makes sense. So far I've
> tested it with droid 4 for it's modem to provide char devices for AT
> ports, modem PM support, and serdev drivers for GNSS and Alsa ASoC.
> 
> I'll be posting the related MFD, GNSS and Alsa ASoC drivers separately.
> For reference, the MFD driver is at [0], the GNSS driver at [1], and
> the Alsa ASoC driver at [2] below.

I have applied the first two patches to my tree, as those are nice
cleanups.

The last one I want some feedback from the serdev developers to verify
all is set up properly, and Johan, to see if this ends up conflicting
with the gnss code, as that would not be good.

thanks,

greg k-h

^ permalink raw reply

* Re: [PATCH] 8250_pci.c: Update NI specific devices class to multi serial
From: Greg KH @ 2019-01-18 11:57 UTC (permalink / raw)
  To: Guan Yung Tseng; +Cc: linux-serial, linux-kernel
In-Reply-To: <1547475005-23435-1-git-send-email-guan.yung.tseng@ni.com>

On Mon, Jan 14, 2019 at 10:10:05PM +0800, Guan Yung Tseng wrote:
> Modified NI devices class to PCI_CLASS_COMMUNICATION_MULTISERIAL.
> The reason of doing this is because all NI multi port serial cards
> use PCI_CLASS_COMMUNICATION_OTHER class and thus fail the
> serial_pci_is_class_communication test added in the commit 7d8905d06405
> ("serial: 8250_pci: Enable device after we check black list").
> 
> Signed-off-by: Guan Yung Tseng <guan.yung.tseng@ni.com>
> ---
>  drivers/tty/serial/8250/8250_pci.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/drivers/tty/serial/8250/8250_pci.c b/drivers/tty/serial/8250/8250_pci.c
> index 4986b4a..0949db1 100644
> --- a/drivers/tty/serial/8250/8250_pci.c
> +++ b/drivers/tty/serial/8250/8250_pci.c
> @@ -663,6 +663,13 @@ static int pci_xircom_init(struct pci_dev *dev)
>  	return 0;
>  }
>  
> +static int pci_ni_probe(struct pci_dev *dev)
> +{
> +	dev->class = PCI_CLASS_COMMUNICATION_MULTISERIAL << 8 |
> +			(dev->class & 0xff);

As Christoph said, this looks really odd.  This field comes from the
PCI structure on the device, it should not be modified by the kernel.

Unless the device is broken and needs to be fixed up in the kernel?

Also, you have sent 2 different patches here for this type of issue, are
both needed?  If so, please resend both of them as a patch series, with
more explainations in this one as to why you are modifying this field.

I've dropped all pending patches from you from my queue now.

thanks,

greg k-h

^ permalink raw reply

* [PATCH v10 3/3] dt-bindings: dma: uart: rename binding
From: Long Cheng @ 2019-01-18  3:10 UTC (permalink / raw)
  To: Vinod Koul, Randy Dunlap, Rob Herring, Mark Rutland, Ryder Lee,
	Sean Wang, Nicolas Boichat
  Cc: Matthias Brugger, Dan Williams, Greg Kroah-Hartman, Jiri Slaby,
	Sean Wang, dmaengine, devicetree, linux-arm-kernel,
	linux-mediatek, linux-kernel, linux-serial, srv_heupstream,
	Yingjoe Chen, YT Shen, Zhenbao Liu, Long Cheng
In-Reply-To: <1547781016-890-1-git-send-email-long.cheng@mediatek.com>

The filename matches mtk-uart-apdma.c.
So using "mtk-uart-apdma.txt" should be better.

Signed-off-by: Long Cheng <long.cheng@mediatek.com>
---
 .../devicetree/bindings/dma/8250_mtk_dma.txt       |   33 --------------------
 .../devicetree/bindings/dma/mtk-uart-apdma.txt     |   33 ++++++++++++++++++++
 2 files changed, 33 insertions(+), 33 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/dma/8250_mtk_dma.txt
 create mode 100644 Documentation/devicetree/bindings/dma/mtk-uart-apdma.txt

diff --git a/Documentation/devicetree/bindings/dma/8250_mtk_dma.txt b/Documentation/devicetree/bindings/dma/8250_mtk_dma.txt
deleted file mode 100644
index 3fe0961..0000000
--- a/Documentation/devicetree/bindings/dma/8250_mtk_dma.txt
+++ /dev/null
@@ -1,33 +0,0 @@
-* Mediatek UART APDMA Controller
-
-Required properties:
-- compatible should contain:
-  * "mediatek,mt2712-uart-dma" for MT2712 compatible APDMA
-  * "mediatek,mt6577-uart-dma" for MT6577 and all of the above
-
-- reg: The base address of the APDMA register bank.
-
-- interrupts: A single interrupt specifier.
-
-- clocks : Must contain an entry for each entry in clock-names.
-  See ../clocks/clock-bindings.txt for details.
-- clock-names: The APDMA clock for register accesses
-
-Examples:
-
-	apdma: dma-controller@11000380 {
-		compatible = "mediatek,mt2712-uart-dma";
-		reg = <0 0x11000380 0 0x400>;
-		interrupts = <GIC_SPI 63 IRQ_TYPE_LEVEL_LOW>,
-			     <GIC_SPI 64 IRQ_TYPE_LEVEL_LOW>,
-			     <GIC_SPI 65 IRQ_TYPE_LEVEL_LOW>,
-			     <GIC_SPI 66 IRQ_TYPE_LEVEL_LOW>,
-			     <GIC_SPI 67 IRQ_TYPE_LEVEL_LOW>,
-			     <GIC_SPI 68 IRQ_TYPE_LEVEL_LOW>,
-			     <GIC_SPI 69 IRQ_TYPE_LEVEL_LOW>,
-			     <GIC_SPI 70 IRQ_TYPE_LEVEL_LOW>;
-		clocks = <&pericfg CLK_PERI_AP_DMA>;
-		clock-names = "apdma";
-		#dma-cells = <1>;
-	};
-
diff --git a/Documentation/devicetree/bindings/dma/mtk-uart-apdma.txt b/Documentation/devicetree/bindings/dma/mtk-uart-apdma.txt
new file mode 100644
index 0000000..3fe0961
--- /dev/null
+++ b/Documentation/devicetree/bindings/dma/mtk-uart-apdma.txt
@@ -0,0 +1,33 @@
+* Mediatek UART APDMA Controller
+
+Required properties:
+- compatible should contain:
+  * "mediatek,mt2712-uart-dma" for MT2712 compatible APDMA
+  * "mediatek,mt6577-uart-dma" for MT6577 and all of the above
+
+- reg: The base address of the APDMA register bank.
+
+- interrupts: A single interrupt specifier.
+
+- clocks : Must contain an entry for each entry in clock-names.
+  See ../clocks/clock-bindings.txt for details.
+- clock-names: The APDMA clock for register accesses
+
+Examples:
+
+	apdma: dma-controller@11000380 {
+		compatible = "mediatek,mt2712-uart-dma";
+		reg = <0 0x11000380 0 0x400>;
+		interrupts = <GIC_SPI 63 IRQ_TYPE_LEVEL_LOW>,
+			     <GIC_SPI 64 IRQ_TYPE_LEVEL_LOW>,
+			     <GIC_SPI 65 IRQ_TYPE_LEVEL_LOW>,
+			     <GIC_SPI 66 IRQ_TYPE_LEVEL_LOW>,
+			     <GIC_SPI 67 IRQ_TYPE_LEVEL_LOW>,
+			     <GIC_SPI 68 IRQ_TYPE_LEVEL_LOW>,
+			     <GIC_SPI 69 IRQ_TYPE_LEVEL_LOW>,
+			     <GIC_SPI 70 IRQ_TYPE_LEVEL_LOW>;
+		clocks = <&pericfg CLK_PERI_AP_DMA>;
+		clock-names = "apdma";
+		#dma-cells = <1>;
+	};
+
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH v10 2/3] arm: dts: mt2712: add uart APDMA to device tree
From: Long Cheng @ 2019-01-18  3:10 UTC (permalink / raw)
  To: Vinod Koul, Randy Dunlap, Rob Herring, Mark Rutland, Ryder Lee,
	Sean Wang, Nicolas Boichat
  Cc: Matthias Brugger, Dan Williams, Greg Kroah-Hartman, Jiri Slaby,
	Sean Wang, dmaengine, devicetree, linux-arm-kernel,
	linux-mediatek, linux-kernel, linux-serial, srv_heupstream,
	Yingjoe Chen, YT Shen, Zhenbao Liu, Long Cheng
In-Reply-To: <1547781016-890-1-git-send-email-long.cheng@mediatek.com>

1. add uart APDMA controller device node
2. add uart 0/1/2/3/4/5 DMA function

Signed-off-by: Long Cheng <long.cheng@mediatek.com>
---
 arch/arm64/boot/dts/mediatek/mt2712e.dtsi |   51 +++++++++++++++++++++++++++++
 1 file changed, 51 insertions(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt2712e.dtsi b/arch/arm64/boot/dts/mediatek/mt2712e.dtsi
index ee627a7..3469a6c 100644
--- a/arch/arm64/boot/dts/mediatek/mt2712e.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt2712e.dtsi
@@ -298,6 +298,9 @@
 		interrupts = <GIC_SPI 127 IRQ_TYPE_LEVEL_LOW>;
 		clocks = <&baud_clk>, <&sys_clk>;
 		clock-names = "baud", "bus";
+		dmas = <&apdma 10
+			&apdma 11>;
+		dma-names = "tx", "rx";
 		status = "disabled";
 	};
 
@@ -346,6 +349,39 @@
 			 (GIC_CPU_MASK_RAW(0x13) | IRQ_TYPE_LEVEL_HIGH)>;
 	};
 
+	apdma: dma-controller@11000400 {
+		compatible = "mediatek,mt2712-uart-dma",
+			     "mediatek,mt6577-uart-dma";
+		reg = <0 0x11000400 0 0x80>,
+		      <0 0x11000480 0 0x80>,
+		      <0 0x11000500 0 0x80>,
+		      <0 0x11000580 0 0x80>,
+		      <0 0x11000600 0 0x80>,
+		      <0 0x11000680 0 0x80>,
+		      <0 0x11000700 0 0x80>,
+		      <0 0x11000780 0 0x80>,
+		      <0 0x11000800 0 0x80>,
+		      <0 0x11000880 0 0x80>,
+		      <0 0x11000900 0 0x80>,
+		      <0 0x11000980 0 0x80>;
+		interrupts = <GIC_SPI 103 IRQ_TYPE_LEVEL_LOW>,
+			     <GIC_SPI 104 IRQ_TYPE_LEVEL_LOW>,
+			     <GIC_SPI 105 IRQ_TYPE_LEVEL_LOW>,
+			     <GIC_SPI 106 IRQ_TYPE_LEVEL_LOW>,
+			     <GIC_SPI 107 IRQ_TYPE_LEVEL_LOW>,
+			     <GIC_SPI 108 IRQ_TYPE_LEVEL_LOW>,
+			     <GIC_SPI 109 IRQ_TYPE_LEVEL_LOW>,
+			     <GIC_SPI 110 IRQ_TYPE_LEVEL_LOW>,
+			     <GIC_SPI 111 IRQ_TYPE_LEVEL_LOW>,
+			     <GIC_SPI 112 IRQ_TYPE_LEVEL_LOW>,
+			     <GIC_SPI 113 IRQ_TYPE_LEVEL_LOW>,
+			     <GIC_SPI 114 IRQ_TYPE_LEVEL_LOW>;
+		dma-requests = <12>;
+		clocks = <&pericfg CLK_PERI_AP_DMA>;
+		clock-names = "apdma";
+		#dma-cells = <1>;
+	};
+
 	auxadc: adc@11001000 {
 		compatible = "mediatek,mt2712-auxadc";
 		reg = <0 0x11001000 0 0x1000>;
@@ -362,6 +398,9 @@
 		interrupts = <GIC_SPI 91 IRQ_TYPE_LEVEL_LOW>;
 		clocks = <&baud_clk>, <&sys_clk>;
 		clock-names = "baud", "bus";
+		dmas = <&apdma 0
+			&apdma 1>;
+		dma-names = "tx", "rx";
 		status = "disabled";
 	};
 
@@ -372,6 +411,9 @@
 		interrupts = <GIC_SPI 92 IRQ_TYPE_LEVEL_LOW>;
 		clocks = <&baud_clk>, <&sys_clk>;
 		clock-names = "baud", "bus";
+		dmas = <&apdma 2
+			&apdma 3>;
+		dma-names = "tx", "rx";
 		status = "disabled";
 	};
 
@@ -382,6 +424,9 @@
 		interrupts = <GIC_SPI 93 IRQ_TYPE_LEVEL_LOW>;
 		clocks = <&baud_clk>, <&sys_clk>;
 		clock-names = "baud", "bus";
+		dmas = <&apdma 4
+			&apdma 5>;
+		dma-names = "tx", "rx";
 		status = "disabled";
 	};
 
@@ -392,6 +437,9 @@
 		interrupts = <GIC_SPI 94 IRQ_TYPE_LEVEL_LOW>;
 		clocks = <&baud_clk>, <&sys_clk>;
 		clock-names = "baud", "bus";
+		dmas = <&apdma 6
+			&apdma 7>;
+		dma-names = "tx", "rx";
 		status = "disabled";
 	};
 
@@ -402,6 +450,9 @@
 		interrupts = <GIC_SPI 126 IRQ_TYPE_LEVEL_LOW>;
 		clocks = <&baud_clk>, <&sys_clk>;
 		clock-names = "baud", "bus";
+		dmas = <&apdma 8
+			&apdma 9>;
+		dma-names = "tx", "rx";
 		status = "disabled";
 	};
 
-- 
1.7.9.5

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox