Re: linux-next: manual merge of the char-misc tree with Linus' tree

Greg KH [off-list ref] writes:

On Fri, Aug 27, 2021 at 04:49:04PM +1000, Stephen Rothwell wrote:

quoted

Hi all,

Today's linux-next merge of the char-misc tree got conflicts in:

  drivers/bus/mhi/core/main.c
  net/qrtr/mhi.c

between commit:

  9ebc2758d0bb ("Revert "net: really fix the build..."")

from the origin tree and commit:

  0092a1e3f763 ("bus: mhi: Add inbound buffers allocation flag")

from the char-misc tree.

I fixed it up (the commit in Linus' tree is basically a revert of the
char-misc tree, so I effectively reverted the latter) and can carry the
fix as necessary. This is now fixed as far as linux-next is concerned,
but any non trivial conflicts should be mentioned to your upstream
maintainer when your tree is submitted for merging.  You may also want
to consider cooperating with the maintainer of the conflicting tree to
minimise any particularly complex conflicts.

Hm, what should I do in my tree here?

Kalle, what commit should I make in the char-misc tree now to handle
this issue, and make the merge with Linus's tree "simple"?  Or any other
ideas?

Good news first, I tested next-20210827 with ath11k (using QCA6390) and
it's working fine. So Stephen solved the conflict correctly, at least
from ath11k point of view. And my suggestion is that Linus should do the
same resolution when merging char-misc-next (after he has merged
net-next).

I'll explain that in detail below. But do note that I'm not familiar
with the MHI subsystem and the MHI folks really should look at this in
detail to make sure no new bugs are introduced! I did the conflict
resolution myself and at least ath11k works after this resolution.

In my test merge I first used Linus' tree as of today as the baseline.
I first merged net-next and it went without conflicts. After that I
merged char-misc-next and got conflicts in two files:

$ git merge char-misc/char-misc-next
Auto-merging net/qrtr/mhi.c
CONFLICT (content): Merge conflict in net/qrtr/mhi.c
Auto-merging include/linux/mhi.h
Auto-merging drivers/bus/mhi/pci_generic.c
Auto-merging drivers/bus/mhi/core/main.c
CONFLICT (content): Merge conflict in drivers/bus/mhi/core/main.c
Auto-merging drivers/bus/mhi/core/internal.h
Auto-merging drivers/bus/fsl-mc/fsl-mc-bus.c
Removing Documentation/devicetree/bindings/phy/ti,phy-am654-serdes.txt
Removing Documentation/devicetree/bindings/phy/fsl,imx8mq-usb-phy.txt
Removing Documentation/devicetree/bindings/misc/ge-achc.txt
Removing Documentation/devicetree/bindings/firmware/xilinx/xlnx,zynqmp-firmware.txt
Recorded preimage for 'drivers/bus/mhi/core/main.c'
Recorded preimage for 'net/qrtr/mhi.c'
Automatic merge failed; fix conflicts and then commit the result.

Luckily the conflicts are simple:

diff --cc drivers/bus/mhi/core/main.c
index fc9196f11cb7,c01ec2fef02c..000000000000

--- a/drivers/bus/mhi/core/main.c
+++ b/drivers/bus/mhi/core/main.c

@@@ -1455,6 -1455,9 +1455,12 @@@ int mhi_prepare_channel(struct mhi_cont
        if (ret)
                goto error_pm_state;
  
++<<<<<<< HEAD
++=======
+       if (mhi_chan->dir == DMA_FROM_DEVICE)
+               mhi_chan->pre_alloc = !!(flags & MHI_CH_INBOUND_ALLOC_BUFS);
+ 
++>>>>>>> char-misc/char-misc-next
        /* Pre-allocate buffer for xfer ring */
        if (mhi_chan->pre_alloc) {
                int nr_el = get_nr_avail_ring_elements(mhi_cntrl,
diff --cc net/qrtr/mhi.c
index fa611678af05,c609cb724c25..000000000000

--- a/net/qrtr/mhi.c
+++ b/net/qrtr/mhi.c

@@@ -79,7 -84,7 +79,11 @@@ static int qcom_mhi_qrtr_probe(struct m
        int rc;
  
        /* start channels */
++<<<<<<< HEAD
 +      rc = mhi_prepare_for_transfer(mhi_dev);
++=======
+       rc = mhi_prepare_for_transfer(mhi_dev, MHI_CH_INBOUND_ALLOC_BUFS);
++>>>>>>> char-misc/char-misc-next
        if (rc)
                return rc;

In both cases just take the first hunk from HEAD and remove the second
hunk from char-misc-next. After the conflict resolution 'git diff'
should show:

diff --git a/drivers/bus/mhi/core/main.c b/drivers/bus/mhi/core/main.c
index 88dba230f406..b15c5bc37dd4 100644
--- a/drivers/bus/mhi/core/main.c
+++ b/drivers/bus/mhi/core/main.c

@@ -1455,9 +1455,6 @@ int mhi_prepare_channel(struct mhi_controller *mhi_cntrl,
        if (ret)
                goto error_pm_state;
 
-       if (mhi_chan->dir == DMA_FROM_DEVICE)
-               mhi_chan->pre_alloc = !!(flags & MHI_CH_INBOUND_ALLOC_BUFS);
-
        /* Pre-allocate buffer for xfer ring */
        if (mhi_chan->pre_alloc) {
                int nr_el = get_nr_avail_ring_elements(mhi_cntrl,

Greg, does this help? Stephen, do you have any advice how to handle
this?

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Hi Kalle, Stephen,

On Fri, Aug 27, 2021 at 07:24:49PM +0300, Kalle Valo wrote:

Greg KH [off-list ref] writes:

[...]

I'll explain that in detail below. But do note that I'm not familiar
with the MHI subsystem and the MHI folks really should look at this in
detail to make sure no new bugs are introduced! I did the conflict
resolution myself and at least ath11k works after this resolution.

I wanted to fix this thing before breaking for vacation but ended up responding
to it in the middle.

In my test merge I first used Linus' tree as of today as the baseline.
I first merged net-next and it went without conflicts. After that I
merged char-misc-next and got conflicts in two files:

[...]

Sorry, this fix won't work. I've done the conflict resolution in
mhi-conflict-fix [1] branch for reference.

The MHI diffs are below:

diff --git a/drivers/bus/mhi/core/main.c b/drivers/bus/mhi/core/main.c
index fc9196f11cb7d..c01ec2fef02ce 100644
--- a/drivers/bus/mhi/core/main.c
+++ b/drivers/bus/mhi/core/main.c

@@ -193,7 +193,7 @@ int mhi_map_single_no_bb(struct mhi_controller *mhi_cntrl,
 int mhi_map_single_use_bb(struct mhi_controller *mhi_cntrl,
 			  struct mhi_buf_info *buf_info)
 {
-	void *buf = mhi_alloc_coherent(mhi_cntrl, buf_info->len,
+	void *buf = dma_alloc_coherent(mhi_cntrl->cntrl_dev, buf_info->len,
 				       &buf_info->p_addr, GFP_ATOMIC);
 
 	if (!buf)

@@ -220,8 +220,8 @@ void mhi_unmap_single_use_bb(struct mhi_controller *mhi_cntrl,
 	if (buf_info->dir == DMA_FROM_DEVICE)
 		memcpy(buf_info->v_addr, buf_info->bb_addr, buf_info->len);
 
-	mhi_free_coherent(mhi_cntrl, buf_info->len, buf_info->bb_addr,
-			  buf_info->p_addr);
+	dma_free_coherent(mhi_cntrl->cntrl_dev, buf_info->len,
+			  buf_info->bb_addr, buf_info->p_addr);
 }
 
 static int get_nr_avail_ring_elements(struct mhi_controller *mhi_cntrl,

@@ -1430,7 +1430,7 @@ exit_unprepare_channel:
 }
 
 int mhi_prepare_channel(struct mhi_controller *mhi_cntrl,
-			struct mhi_chan *mhi_chan)
+			struct mhi_chan *mhi_chan, unsigned int flags)
 {
 	int ret = 0;
 	struct device *dev = &mhi_chan->mhi_dev->dev;

@@ -1455,6 +1455,9 @@ int mhi_prepare_channel(struct mhi_controller *mhi_cntrl,
 	if (ret)
 		goto error_pm_state;
 
+	if (mhi_chan->dir == DMA_FROM_DEVICE)
+		mhi_chan->pre_alloc = !!(flags & MHI_CH_INBOUND_ALLOC_BUFS);
+
 	/* Pre-allocate buffer for xfer ring */
 	if (mhi_chan->pre_alloc) {
 		int nr_el = get_nr_avail_ring_elements(mhi_cntrl,

@@ -1610,7 +1613,7 @@ void mhi_reset_chan(struct mhi_controller *mhi_cntrl, struct mhi_chan *mhi_chan)
 }
 
 /* Move channel to start state */
-int mhi_prepare_for_transfer(struct mhi_device *mhi_dev)
+int mhi_prepare_for_transfer(struct mhi_device *mhi_dev, unsigned int flags)
 {
 	int ret, dir;
 	struct mhi_controller *mhi_cntrl = mhi_dev->mhi_cntrl;

@@ -1621,7 +1624,7 @@ int mhi_prepare_for_transfer(struct mhi_device *mhi_dev)
 		if (!mhi_chan)
 			continue;
 
-		ret = mhi_prepare_channel(mhi_cntrl, mhi_chan);
+		ret = mhi_prepare_channel(mhi_cntrl, mhi_chan, flags);
 		if (ret)
 			goto error_open_chan;
 	}

diff --git a/net/qrtr/mhi.c b/net/qrtr/mhi.c
index fa611678af052..29b4fa3b72abf 100644
--- a/net/qrtr/mhi.c
+++ b/net/qrtr/mhi.c

@@ -79,7 +79,7 @@ static int qcom_mhi_qrtr_probe(struct mhi_device *mhi_dev,
 	int rc;
 
 	/* start channels */
-	rc = mhi_prepare_for_transfer(mhi_dev);
+	rc = mhi_prepare_for_transfer(mhi_dev, MHI_CH_INBOUND_ALLOC_BUFS);
 	if (rc)
 		return rc;

But this can be avoided if the below commit is reverted in char-misc-next:

0092a1e3f763 ("bus: mhi: Add inbound buffers allocation flag")

I won't have any objection to that (the original patch can land in v5.16) and
if Greg prefers that, I can send a quick revert patch.

Please let me know!

NOTE: I haven't tested these two solutions as I'm still on vacation. Kalle,
could you please test them to make sure I haven't missed anything?

Thanks,
Mani

[1] https://git.kernel.org/pub/scm/linux/kernel/git/mani/mhi.git/log/?h=mhi-conflict-fix

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches