| Message ID | 20220830171147.24338-1-quic_jjohnson@quicinc.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | [v5] bus: mhi: host: make mhi_controller_config::event_cfg const | expand |
On Tue, Aug 30, 2022 at 10:11:47AM -0700, Jeff Johnson wrote: > Currently the event_cfg pointer in struct mhi_controller_config is not > defined as a const pointer. This prevents clients from registering a > read-only configuration unless they use a typecast. Since the > event_cfg should not be modified once registered, add the const > qualifier to event_cfg. This is aligned with the definition of ch_cfg. > > Signed-off-by: Jeff Johnson <quic_jjohnson@quicinc.com> Reviewed-by: Manivannan Sadhasivam <mani@kernel.org> Thanks, Mani > --- > v5: corrected mhi_foxconn_sdx55_events[] > > v4: updated subject, rebased to v6.0-rc3 > > v3: added pci_generic.c change > > v2: added S-O-B > > drivers/bus/mhi/host/pci_generic.c | 14 +++++++------- > include/linux/mhi.h | 2 +- > 2 files changed, 8 insertions(+), 8 deletions(-) > > diff --git a/drivers/bus/mhi/host/pci_generic.c b/drivers/bus/mhi/host/pci_generic.c > index 9e545f2a5a26..0db437ac3ba4 100644 > --- a/drivers/bus/mhi/host/pci_generic.c > +++ b/drivers/bus/mhi/host/pci_generic.c > @@ -238,7 +238,7 @@ static const struct mhi_channel_config modem_qcom_v1_mhi_channels[] = { > MHI_CHANNEL_CONFIG_HW_DL(101, "IP_HW0", 128, 3), > }; > > -static struct mhi_event_config modem_qcom_v1_mhi_events[] = { > +static const struct mhi_event_config modem_qcom_v1_mhi_events[] = { > /* first ring is control+data ring */ > MHI_EVENT_CONFIG_CTRL(0, 64), > /* DIAG dedicated event ring */ > @@ -305,7 +305,7 @@ static const struct mhi_channel_config mhi_quectel_em1xx_channels[] = { > MHI_CHANNEL_CONFIG_HW_DL(101, "IP_HW0_MBIM", 128, 3), > }; > > -static struct mhi_event_config mhi_quectel_em1xx_events[] = { > +static const struct mhi_event_config mhi_quectel_em1xx_events[] = { > MHI_EVENT_CONFIG_CTRL(0, 128), > MHI_EVENT_CONFIG_DATA(1, 128), > MHI_EVENT_CONFIG_HW_DATA(2, 1024, 100), > @@ -344,7 +344,7 @@ static const struct mhi_channel_config mhi_foxconn_sdx55_channels[] = { > MHI_CHANNEL_CONFIG_HW_DL(101, "IP_HW0_MBIM", 128, 3), > }; > > -static struct mhi_event_config mhi_foxconn_sdx55_events[] = { > +static const struct mhi_event_config mhi_foxconn_sdx55_events[] = { > MHI_EVENT_CONFIG_CTRL(0, 128), > MHI_EVENT_CONFIG_DATA(1, 128), > MHI_EVENT_CONFIG_HW_DATA(2, 1024, 100), > @@ -391,7 +391,7 @@ static const struct mhi_channel_config mhi_mv3x_channels[] = { > MHI_CHANNEL_CONFIG_HW_DL(101, "IP_HW0_MBIM", 512, 3), > }; > > -static struct mhi_event_config mhi_mv3x_events[] = { > +static const struct mhi_event_config mhi_mv3x_events[] = { > MHI_EVENT_CONFIG_CTRL(0, 256), > MHI_EVENT_CONFIG_DATA(1, 256), > MHI_EVENT_CONFIG_HW_DATA(2, 1024, 100), > @@ -438,7 +438,7 @@ static const struct mhi_channel_config mhi_sierra_em919x_channels[] = { > MHI_CHANNEL_CONFIG_HW_DL(101, "IP_HW0", 512, 2), > }; > > -static struct mhi_event_config modem_sierra_em919x_mhi_events[] = { > +static const struct mhi_event_config modem_sierra_em919x_mhi_events[] = { > /* first ring is control+data and DIAG ring */ > MHI_EVENT_CONFIG_CTRL(0, 2048), > /* Hardware channels request dedicated hardware event rings */ > @@ -472,7 +472,7 @@ static const struct mhi_channel_config mhi_telit_fn980_hw_v1_channels[] = { > MHI_CHANNEL_CONFIG_HW_DL(101, "IP_HW0", 128, 2), > }; > > -static struct mhi_event_config mhi_telit_fn980_hw_v1_events[] = { > +static const struct mhi_event_config mhi_telit_fn980_hw_v1_events[] = { > MHI_EVENT_CONFIG_CTRL(0, 128), > MHI_EVENT_CONFIG_HW_DATA(1, 1024, 100), > MHI_EVENT_CONFIG_HW_DATA(2, 2048, 101) > @@ -511,7 +511,7 @@ static const struct mhi_channel_config mhi_telit_fn990_channels[] = { > MHI_CHANNEL_CONFIG_HW_DL(101, "IP_HW0_MBIM", 128, 3), > }; > > -static struct mhi_event_config mhi_telit_fn990_events[] = { > +static const struct mhi_event_config mhi_telit_fn990_events[] = { > MHI_EVENT_CONFIG_CTRL(0, 128), > MHI_EVENT_CONFIG_DATA(1, 128), > MHI_EVENT_CONFIG_HW_DATA(2, 1024, 100), > diff --git a/include/linux/mhi.h b/include/linux/mhi.h > index a5441ad33c74..ada2f18af4d6 100644 > --- a/include/linux/mhi.h > +++ b/include/linux/mhi.h > @@ -281,7 +281,7 @@ struct mhi_controller_config { > u32 num_channels; > const struct mhi_channel_config *ch_cfg; > u32 num_events; > - struct mhi_event_config *event_cfg; > + const struct mhi_event_config *event_cfg; > bool use_bounce_buf; > bool m2_no_db; > }; > -- > 2.37.0 > >
On Tue, Aug 30, 2022 at 10:11:47AM -0700, Jeff Johnson wrote: > Currently the event_cfg pointer in struct mhi_controller_config is not > defined as a const pointer. This prevents clients from registering a > read-only configuration unless they use a typecast. Since the > event_cfg should not be modified once registered, add the const > qualifier to event_cfg. This is aligned with the definition of ch_cfg. > > Signed-off-by: Jeff Johnson <quic_jjohnson@quicinc.com> Applied to mhi-next! Thanks, Mani > --- > v5: corrected mhi_foxconn_sdx55_events[] > > v4: updated subject, rebased to v6.0-rc3 > > v3: added pci_generic.c change > > v2: added S-O-B > > drivers/bus/mhi/host/pci_generic.c | 14 +++++++------- > include/linux/mhi.h | 2 +- > 2 files changed, 8 insertions(+), 8 deletions(-) > > diff --git a/drivers/bus/mhi/host/pci_generic.c b/drivers/bus/mhi/host/pci_generic.c > index 9e545f2a5a26..0db437ac3ba4 100644 > --- a/drivers/bus/mhi/host/pci_generic.c > +++ b/drivers/bus/mhi/host/pci_generic.c > @@ -238,7 +238,7 @@ static const struct mhi_channel_config modem_qcom_v1_mhi_channels[] = { > MHI_CHANNEL_CONFIG_HW_DL(101, "IP_HW0", 128, 3), > }; > > -static struct mhi_event_config modem_qcom_v1_mhi_events[] = { > +static const struct mhi_event_config modem_qcom_v1_mhi_events[] = { > /* first ring is control+data ring */ > MHI_EVENT_CONFIG_CTRL(0, 64), > /* DIAG dedicated event ring */ > @@ -305,7 +305,7 @@ static const struct mhi_channel_config mhi_quectel_em1xx_channels[] = { > MHI_CHANNEL_CONFIG_HW_DL(101, "IP_HW0_MBIM", 128, 3), > }; > > -static struct mhi_event_config mhi_quectel_em1xx_events[] = { > +static const struct mhi_event_config mhi_quectel_em1xx_events[] = { > MHI_EVENT_CONFIG_CTRL(0, 128), > MHI_EVENT_CONFIG_DATA(1, 128), > MHI_EVENT_CONFIG_HW_DATA(2, 1024, 100), > @@ -344,7 +344,7 @@ static const struct mhi_channel_config mhi_foxconn_sdx55_channels[] = { > MHI_CHANNEL_CONFIG_HW_DL(101, "IP_HW0_MBIM", 128, 3), > }; > > -static struct mhi_event_config mhi_foxconn_sdx55_events[] = { > +static const struct mhi_event_config mhi_foxconn_sdx55_events[] = { > MHI_EVENT_CONFIG_CTRL(0, 128), > MHI_EVENT_CONFIG_DATA(1, 128), > MHI_EVENT_CONFIG_HW_DATA(2, 1024, 100), > @@ -391,7 +391,7 @@ static const struct mhi_channel_config mhi_mv3x_channels[] = { > MHI_CHANNEL_CONFIG_HW_DL(101, "IP_HW0_MBIM", 512, 3), > }; > > -static struct mhi_event_config mhi_mv3x_events[] = { > +static const struct mhi_event_config mhi_mv3x_events[] = { > MHI_EVENT_CONFIG_CTRL(0, 256), > MHI_EVENT_CONFIG_DATA(1, 256), > MHI_EVENT_CONFIG_HW_DATA(2, 1024, 100), > @@ -438,7 +438,7 @@ static const struct mhi_channel_config mhi_sierra_em919x_channels[] = { > MHI_CHANNEL_CONFIG_HW_DL(101, "IP_HW0", 512, 2), > }; > > -static struct mhi_event_config modem_sierra_em919x_mhi_events[] = { > +static const struct mhi_event_config modem_sierra_em919x_mhi_events[] = { > /* first ring is control+data and DIAG ring */ > MHI_EVENT_CONFIG_CTRL(0, 2048), > /* Hardware channels request dedicated hardware event rings */ > @@ -472,7 +472,7 @@ static const struct mhi_channel_config mhi_telit_fn980_hw_v1_channels[] = { > MHI_CHANNEL_CONFIG_HW_DL(101, "IP_HW0", 128, 2), > }; > > -static struct mhi_event_config mhi_telit_fn980_hw_v1_events[] = { > +static const struct mhi_event_config mhi_telit_fn980_hw_v1_events[] = { > MHI_EVENT_CONFIG_CTRL(0, 128), > MHI_EVENT_CONFIG_HW_DATA(1, 1024, 100), > MHI_EVENT_CONFIG_HW_DATA(2, 2048, 101) > @@ -511,7 +511,7 @@ static const struct mhi_channel_config mhi_telit_fn990_channels[] = { > MHI_CHANNEL_CONFIG_HW_DL(101, "IP_HW0_MBIM", 128, 3), > }; > > -static struct mhi_event_config mhi_telit_fn990_events[] = { > +static const struct mhi_event_config mhi_telit_fn990_events[] = { > MHI_EVENT_CONFIG_CTRL(0, 128), > MHI_EVENT_CONFIG_DATA(1, 128), > MHI_EVENT_CONFIG_HW_DATA(2, 1024, 100), > diff --git a/include/linux/mhi.h b/include/linux/mhi.h > index a5441ad33c74..ada2f18af4d6 100644 > --- a/include/linux/mhi.h > +++ b/include/linux/mhi.h > @@ -281,7 +281,7 @@ struct mhi_controller_config { > u32 num_channels; > const struct mhi_channel_config *ch_cfg; > u32 num_events; > - struct mhi_event_config *event_cfg; > + const struct mhi_event_config *event_cfg; > bool use_bounce_buf; > bool m2_no_db; > }; > -- > 2.37.0 > >
Manivannan Sadhasivam <mani@kernel.org> writes: > On Tue, Aug 30, 2022 at 10:11:47AM -0700, Jeff Johnson wrote: >> Currently the event_cfg pointer in struct mhi_controller_config is not >> defined as a const pointer. This prevents clients from registering a >> read-only configuration unless they use a typecast. Since the >> event_cfg should not be modified once registered, add the const >> qualifier to event_cfg. This is aligned with the definition of ch_cfg. >> >> Signed-off-by: Jeff Johnson <quic_jjohnson@quicinc.com> > > Applied to mhi-next! BTW what happened to this patch? I cannot find it anywhere.
On Tue, Nov 08, 2022 at 03:33:35PM +0200, Kalle Valo wrote: > Manivannan Sadhasivam <mani@kernel.org> writes: > > > On Tue, Aug 30, 2022 at 10:11:47AM -0700, Jeff Johnson wrote: > >> Currently the event_cfg pointer in struct mhi_controller_config is not > >> defined as a const pointer. This prevents clients from registering a > >> read-only configuration unless they use a typecast. Since the > >> event_cfg should not be modified once registered, add the const > >> qualifier to event_cfg. This is aligned with the definition of ch_cfg. > >> > >> Signed-off-by: Jeff Johnson <quic_jjohnson@quicinc.com> > > > > Applied to mhi-next! > > BTW what happened to this patch? I cannot find it anywhere. > Sorry! I did apply this patch earlier locally but didn't push it to remote. So it got lost. Now pushed! Thanks for spotting. > -- > https://patchwork.kernel.org/project/linux-wireless/list/ > > https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches >
Manivannan Sadhasivam <mani@kernel.org> writes: > On Tue, Nov 08, 2022 at 03:33:35PM +0200, Kalle Valo wrote: >> Manivannan Sadhasivam <mani@kernel.org> writes: >> >> > On Tue, Aug 30, 2022 at 10:11:47AM -0700, Jeff Johnson wrote: >> >> Currently the event_cfg pointer in struct mhi_controller_config is not >> >> defined as a const pointer. This prevents clients from registering a >> >> read-only configuration unless they use a typecast. Since the >> >> event_cfg should not be modified once registered, add the const >> >> qualifier to event_cfg. This is aligned with the definition of ch_cfg. >> >> >> >> Signed-off-by: Jeff Johnson <quic_jjohnson@quicinc.com> >> > >> > Applied to mhi-next! >> >> BTW what happened to this patch? I cannot find it anywhere. >> > > Sorry! I did apply this patch earlier locally but didn't push it to > remote. So it got lost. > > Now pushed! Thanks for spotting. Great, thanks Mani.
On Tue, Nov 08, 2022 at 08:09:02PM +0530, Manivannan Sadhasivam wrote: > On Tue, Nov 08, 2022 at 03:33:35PM +0200, Kalle Valo wrote: > > Manivannan Sadhasivam <mani@kernel.org> writes: > > > > > On Tue, Aug 30, 2022 at 10:11:47AM -0700, Jeff Johnson wrote: > > >> Currently the event_cfg pointer in struct mhi_controller_config is not > > >> defined as a const pointer. This prevents clients from registering a > > >> read-only configuration unless they use a typecast. Since the > > >> event_cfg should not be modified once registered, add the const > > >> qualifier to event_cfg. This is aligned with the definition of ch_cfg. > > >> > > >> Signed-off-by: Jeff Johnson <quic_jjohnson@quicinc.com> > > > > > > Applied to mhi-next! > > > > BTW what happened to this patch? I cannot find it anywhere. > > > > Sorry! I did apply this patch earlier locally but didn't push it to > remote. So it got lost. > > Now pushed! Thanks for spotting. > Dropped the patch now because of this: https://lore.kernel.org/lkml/20221109151637.67be60f8@canb.auug.org.au/ Since we need to modify event_cfg for using the shared IRQ, I don't think the patch is applicable. Maybe that's the reason I dropped it earlier also, but forgot to share it in mailing list. Thanks, Mani > > -- > > https://patchwork.kernel.org/project/linux-wireless/list/ > > > > https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches > > > > -- > மணிவண்ணன் சதாசிவம் >
On 11/8/2022 11:45 PM, Manivannan Sadhasivam wrote: > Dropped the patch now because of this: https://lore.kernel.org/lkml/20221109151637.67be60f8@canb.auug.org.au/ > > Since we need to modify event_cfg for using the shared IRQ, I don't think the > patch is applicable. Maybe that's the reason I dropped it earlier also, but > forgot to share it in mailing list. > > Thanks, > Mani yes, that was the reason it was dropped before. Perhaps this code can be revisited so that the event_cfg doesn't need to be modified for this one case, but I personally don't have time for that now.
diff --git a/drivers/bus/mhi/host/pci_generic.c b/drivers/bus/mhi/host/pci_generic.c index 9e545f2a5a26..0db437ac3ba4 100644 --- a/drivers/bus/mhi/host/pci_generic.c +++ b/drivers/bus/mhi/host/pci_generic.c @@ -238,7 +238,7 @@ static const struct mhi_channel_config modem_qcom_v1_mhi_channels[] = { MHI_CHANNEL_CONFIG_HW_DL(101, "IP_HW0", 128, 3), }; -static struct mhi_event_config modem_qcom_v1_mhi_events[] = { +static const struct mhi_event_config modem_qcom_v1_mhi_events[] = { /* first ring is control+data ring */ MHI_EVENT_CONFIG_CTRL(0, 64), /* DIAG dedicated event ring */ @@ -305,7 +305,7 @@ static const struct mhi_channel_config mhi_quectel_em1xx_channels[] = { MHI_CHANNEL_CONFIG_HW_DL(101, "IP_HW0_MBIM", 128, 3), }; -static struct mhi_event_config mhi_quectel_em1xx_events[] = { +static const struct mhi_event_config mhi_quectel_em1xx_events[] = { MHI_EVENT_CONFIG_CTRL(0, 128), MHI_EVENT_CONFIG_DATA(1, 128), MHI_EVENT_CONFIG_HW_DATA(2, 1024, 100), @@ -344,7 +344,7 @@ static const struct mhi_channel_config mhi_foxconn_sdx55_channels[] = { MHI_CHANNEL_CONFIG_HW_DL(101, "IP_HW0_MBIM", 128, 3), }; -static struct mhi_event_config mhi_foxconn_sdx55_events[] = { +static const struct mhi_event_config mhi_foxconn_sdx55_events[] = { MHI_EVENT_CONFIG_CTRL(0, 128), MHI_EVENT_CONFIG_DATA(1, 128), MHI_EVENT_CONFIG_HW_DATA(2, 1024, 100), @@ -391,7 +391,7 @@ static const struct mhi_channel_config mhi_mv3x_channels[] = { MHI_CHANNEL_CONFIG_HW_DL(101, "IP_HW0_MBIM", 512, 3), }; -static struct mhi_event_config mhi_mv3x_events[] = { +static const struct mhi_event_config mhi_mv3x_events[] = { MHI_EVENT_CONFIG_CTRL(0, 256), MHI_EVENT_CONFIG_DATA(1, 256), MHI_EVENT_CONFIG_HW_DATA(2, 1024, 100), @@ -438,7 +438,7 @@ static const struct mhi_channel_config mhi_sierra_em919x_channels[] = { MHI_CHANNEL_CONFIG_HW_DL(101, "IP_HW0", 512, 2), }; -static struct mhi_event_config modem_sierra_em919x_mhi_events[] = { +static const struct mhi_event_config modem_sierra_em919x_mhi_events[] = { /* first ring is control+data and DIAG ring */ MHI_EVENT_CONFIG_CTRL(0, 2048), /* Hardware channels request dedicated hardware event rings */ @@ -472,7 +472,7 @@ static const struct mhi_channel_config mhi_telit_fn980_hw_v1_channels[] = { MHI_CHANNEL_CONFIG_HW_DL(101, "IP_HW0", 128, 2), }; -static struct mhi_event_config mhi_telit_fn980_hw_v1_events[] = { +static const struct mhi_event_config mhi_telit_fn980_hw_v1_events[] = { MHI_EVENT_CONFIG_CTRL(0, 128), MHI_EVENT_CONFIG_HW_DATA(1, 1024, 100), MHI_EVENT_CONFIG_HW_DATA(2, 2048, 101) @@ -511,7 +511,7 @@ static const struct mhi_channel_config mhi_telit_fn990_channels[] = { MHI_CHANNEL_CONFIG_HW_DL(101, "IP_HW0_MBIM", 128, 3), }; -static struct mhi_event_config mhi_telit_fn990_events[] = { +static const struct mhi_event_config mhi_telit_fn990_events[] = { MHI_EVENT_CONFIG_CTRL(0, 128), MHI_EVENT_CONFIG_DATA(1, 128), MHI_EVENT_CONFIG_HW_DATA(2, 1024, 100), diff --git a/include/linux/mhi.h b/include/linux/mhi.h index a5441ad33c74..ada2f18af4d6 100644 --- a/include/linux/mhi.h +++ b/include/linux/mhi.h @@ -281,7 +281,7 @@ struct mhi_controller_config { u32 num_channels; const struct mhi_channel_config *ch_cfg; u32 num_events; - struct mhi_event_config *event_cfg; + const struct mhi_event_config *event_cfg; bool use_bounce_buf; bool m2_no_db; };
Currently the event_cfg pointer in struct mhi_controller_config is not defined as a const pointer. This prevents clients from registering a read-only configuration unless they use a typecast. Since the event_cfg should not be modified once registered, add the const qualifier to event_cfg. This is aligned with the definition of ch_cfg. Signed-off-by: Jeff Johnson <quic_jjohnson@quicinc.com> --- v5: corrected mhi_foxconn_sdx55_events[] v4: updated subject, rebased to v6.0-rc3 v3: added pci_generic.c change v2: added S-O-B drivers/bus/mhi/host/pci_generic.c | 14 +++++++------- include/linux/mhi.h | 2 +- 2 files changed, 8 insertions(+), 8 deletions(-)