| Message ID | 20241128120922.3518582-3-quic_chejiang@quicinc.com |
|---|---|
| State | New |
| Headers | show |
| Series | bluetooth: qca: Add QCA6698 Bluetooth chip | expand |
On 28/11/2024 15:41, Krzysztof Kozlowski wrote: > On 28/11/2024 13:09, Cheng Jiang wrote: >> Add the compatible for the Bluetooth part of the Qualcomm QCA6698 chipset. > > <form letter> > This is a friendly reminder during the review process. > > It seems my or other reviewer's previous comments were not fully > addressed. Maybe the feedback got lost between the quotes, maybe you > just forgot to apply it. Please go back to the previous discussion and > either implement all requested changes or keep discussing them. > > Thank you. > </form letter> > > Respond to the comment and then implement it. > > Also, version your patches correct and provide changelog. This is v2, > not v1. Wait, no, it's even v3 or v4. You just ask us to the same work twice, don't you? Best regards, Krzysztof
Hi Krzysztof, On 11/28/2024 10:41 PM, Krzysztof Kozlowski wrote: > On 28/11/2024 15:41, Krzysztof Kozlowski wrote: >> On 28/11/2024 13:09, Cheng Jiang wrote: >>> Add the compatible for the Bluetooth part of the Qualcomm QCA6698 chipset. >> >> <form letter> >> This is a friendly reminder during the review process. >> >> It seems my or other reviewer's previous comments were not fully >> addressed. Maybe the feedback got lost between the quotes, maybe you >> just forgot to apply it. Please go back to the previous discussion and >> either implement all requested changes or keep discussing them. >> >> Thank you. >> </form letter> >> >> Respond to the comment and then implement it. >> >> Also, version your patches correct and provide changelog. This is v2, >> not v1. > > Wait, no, it's even v3 or v4. You just ask us to the same work twice, > don't you? Sorry for this. So the version number should be increased even solution/implementation is changed? Will follow the rule. > > Best regards, > Krzysztof
Hi Krzysztof, On 11/28/2024 10:41 PM, Krzysztof Kozlowski wrote: > On 28/11/2024 13:09, Cheng Jiang wrote: >> Add the compatible for the Bluetooth part of the Qualcomm QCA6698 chipset. > > <form letter> > This is a friendly reminder during the review process. > > It seems my or other reviewer's previous comments were not fully > addressed. Maybe the feedback got lost between the quotes, maybe you > just forgot to apply it. Please go back to the previous discussion and > either implement all requested changes or keep discussing them. > > Thank you. > </form letter> > > Respond to the comment and then implement it. > > Also, version your patches correct and provide changelog. This is v2, > not v1. > Thank you for the guidance. > Best regards, > Krzysztof
Hi Dmitry, On 11/28/2024 8:58 PM, Dmitry Baryshkov wrote: > On Thu, Nov 28, 2024 at 08:09:21PM +0800, Cheng Jiang wrote: >> Add the compatible for the Bluetooth part of the Qualcomm QCA6698 chipset. > > ... > And you have misssed to explain why do you need to add it and how it is > different from WCN6855. > Got it. I just explain in the dts/driver change, forget to explain here. If use the firmware-name solution, do we still need add the new compatible string for qcom,qca6698-bt here? The driver may not use this string. >> >> Signed-off-by: Cheng Jiang <quic_chejiang@quicinc.com> >> --- >> .../devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml b/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml >> index 7bb68311c..82105382a 100644 >> --- a/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml >> +++ b/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml >> @@ -18,6 +18,7 @@ properties: >> enum: >> - qcom,qca2066-bt >> - qcom,qca6174-bt >> + - qcom,qca6698-bt >> - qcom,qca9377-bt >> - qcom,wcn3988-bt >> - qcom,wcn3990-bt >> @@ -170,6 +171,7 @@ allOf: >> contains: >> enum: >> - qcom,wcn6855-bt >> + - qcom,qca6698-bt >> then: >> required: >> - vddrfacmn-supply >> -- >> 2.25.1 >> >
On 29/11/2024 03:13, Cheng Jiang (IOE) wrote: > Hi Krzysztof, > > On 11/28/2024 10:41 PM, Krzysztof Kozlowski wrote: >> On 28/11/2024 15:41, Krzysztof Kozlowski wrote: >>> On 28/11/2024 13:09, Cheng Jiang wrote: >>>> Add the compatible for the Bluetooth part of the Qualcomm QCA6698 chipset. >>> >>> <form letter> >>> This is a friendly reminder during the review process. >>> >>> It seems my or other reviewer's previous comments were not fully >>> addressed. Maybe the feedback got lost between the quotes, maybe you >>> just forgot to apply it. Please go back to the previous discussion and >>> either implement all requested changes or keep discussing them. >>> >>> Thank you. >>> </form letter> >>> >>> Respond to the comment and then implement it. >>> >>> Also, version your patches correct and provide changelog. This is v2, >>> not v1. >> >> Wait, no, it's even v3 or v4. You just ask us to the same work twice, >> don't you? > Sorry for this. So the version number should be increased even > solution/implementation is changed? Will follow the rule. Yes, because you don't want to send the same for review over and over again. Best regards, Krzysztof
On Fri, Nov 29, 2024 at 10:30:00AM +0800, Cheng Jiang (IOE) wrote: > Hi Dmitry, > > On 11/28/2024 8:58 PM, Dmitry Baryshkov wrote: > > On Thu, Nov 28, 2024 at 08:09:21PM +0800, Cheng Jiang wrote: > >> Add the compatible for the Bluetooth part of the Qualcomm QCA6698 chipset. > > > > ... > > And you have misssed to explain why do you need to add it and how it is > > different from WCN6855. > > > Got it. I just explain in the dts/driver change, forget to explain here. > > If use the firmware-name solution, do we still need add the new compatible > string for qcom,qca6698-bt here? The driver may not use this string. DT describes the hardware. If you want, you can still add new string _and_ use old one as a fallback compatible: "qcom,qca6698-bt", "qcom,wcn6855-bt". > >> > >> Signed-off-by: Cheng Jiang <quic_chejiang@quicinc.com> > >> --- > >> .../devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml | 2 ++ > >> 1 file changed, 2 insertions(+) > >> > >> diff --git a/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml b/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml > >> index 7bb68311c..82105382a 100644 > >> --- a/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml > >> +++ b/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml > >> @@ -18,6 +18,7 @@ properties: > >> enum: > >> - qcom,qca2066-bt > >> - qcom,qca6174-bt > >> + - qcom,qca6698-bt > >> - qcom,qca9377-bt > >> - qcom,wcn3988-bt > >> - qcom,wcn3990-bt > >> @@ -170,6 +171,7 @@ allOf: > >> contains: > >> enum: > >> - qcom,wcn6855-bt > >> + - qcom,qca6698-bt > >> then: > >> required: > >> - vddrfacmn-supply > >> -- > >> 2.25.1 > >> > > >
Hi Dmitry, On 11/30/2024 1:13 AM, Dmitry Baryshkov wrote: > On Fri, Nov 29, 2024 at 10:30:00AM +0800, Cheng Jiang (IOE) wrote: >> Hi Dmitry, >> >> On 11/28/2024 8:58 PM, Dmitry Baryshkov wrote: >>> On Thu, Nov 28, 2024 at 08:09:21PM +0800, Cheng Jiang wrote: >>>> Add the compatible for the Bluetooth part of the Qualcomm QCA6698 chipset. >>> >>> ... >>> And you have misssed to explain why do you need to add it and how it is >>> different from WCN6855. >>> >> Got it. I just explain in the dts/driver change, forget to explain here. >> >> If use the firmware-name solution, do we still need add the new compatible >> string for qcom,qca6698-bt here? The driver may not use this string. > > DT describes the hardware. If you want, you can still add new string > _and_ use old one as a fallback compatible: "qcom,qca6698-bt", > "qcom,wcn6855-bt". Got it. Thanks! > >>>> >>>> Signed-off-by: Cheng Jiang <quic_chejiang@quicinc.com> >>>> --- >>>> .../devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml | 2 ++ >>>> 1 file changed, 2 insertions(+) >>>> >>>> diff --git a/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml b/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml >>>> index 7bb68311c..82105382a 100644 >>>> --- a/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml >>>> +++ b/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml >>>> @@ -18,6 +18,7 @@ properties: >>>> enum: >>>> - qcom,qca2066-bt >>>> - qcom,qca6174-bt >>>> + - qcom,qca6698-bt >>>> - qcom,qca9377-bt >>>> - qcom,wcn3988-bt >>>> - qcom,wcn3990-bt >>>> @@ -170,6 +171,7 @@ allOf: >>>> contains: >>>> enum: >>>> - qcom,wcn6855-bt >>>> + - qcom,qca6698-bt >>>> then: >>>> required: >>>> - vddrfacmn-supply >>>> -- >>>> 2.25.1 >>>> >>> >> >
diff --git a/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml b/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml index 7bb68311c..82105382a 100644 --- a/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml +++ b/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml @@ -18,6 +18,7 @@ properties: enum: - qcom,qca2066-bt - qcom,qca6174-bt + - qcom,qca6698-bt - qcom,qca9377-bt - qcom,wcn3988-bt - qcom,wcn3990-bt @@ -170,6 +171,7 @@ allOf: contains: enum: - qcom,wcn6855-bt + - qcom,qca6698-bt then: required: - vddrfacmn-supply
Add the compatible for the Bluetooth part of the Qualcomm QCA6698 chipset. Signed-off-by: Cheng Jiang <quic_chejiang@quicinc.com> --- .../devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml | 2 ++ 1 file changed, 2 insertions(+)