* Re: [PATCH RFC v2 0/4] wifi: ath10k: support board-specific firmware overrides
From: Dmitry Baryshkov @ 2024-04-05 12:49 UTC (permalink / raw)
To: Kalle Valo
Cc: Jeff Johnson, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Bjorn Andersson, Konrad Dybcio, ath10k, linux-wireless, netdev,
devicetree, linux-arm-msm, Krzysztof Kozlowski
In-Reply-To: <87sf002d8d.fsf@kernel.org>
On Fri, 5 Apr 2024 at 15:41, Kalle Valo <kvalo@kernel.org> wrote:
>
> Dmitry Baryshkov <dmitry.baryshkov@linaro.org> writes:
>
> > On Fri, 5 Apr 2024 at 15:01, Kalle Valo <kvalo@kernel.org> wrote:
> >
> >>
> >> Dmitry Baryshkov <dmitry.baryshkov@linaro.org> writes:
> >>
> >> > On Fri, 8 Mar 2024 at 17:19, Kalle Valo <kvalo@kernel.org> wrote:
> >> >>
> >> >> Dmitry Baryshkov <dmitry.baryshkov@linaro.org> writes:
> >> >>
> >> >> >> To be on the safe side using 'qcom-rb1' makes sense but on the other
> >> >> >> hand that means we need to update linux-firmware (basically add a new
> >> >> >> symlink) everytime a new product is added. But are there going to be
> >> >> >> that many new ath10k based products?
> >> >> >>
> >> >> >> Using 'qcm2290' is easier because for a new product then there only
> >> >> >> needs to be a change in DTS and no need to change anything
> >> >> >> linux-firmware. But here the risk is that if there's actually two
> >> >> >> different ath10k firmware branches for 'qcm2290'. If that ever happens
> >> >> >> (I hope not) I guess we could solve that by adding new 'qcm2290-foo'
> >> >> >> directory?
> >> >> >>
> >> >> >> But I don't really know, thoughts?
> >> >> >
> >> >> > After some thought, I'd suggest to follow approach taken by the rest
> >> >> > of qcom firmware:
> >> >>
> >> >> Can you provide pointers to those cases?
> >> >
> >> > https://gitlab.com/kernel-firmware/linux-firmware/-/tree/main/qcom/sc8280xp/LENOVO/21BX
> >> >
> >> >>
> >> >> > put a default (accepted by non-secured hardware) firmware to SoC dir
> >> >> > and then put a vendor-specific firmware into subdir. If any of such
> >> >> > vendors appear, we might even implement structural fallback: first
> >> >> > look into sdm845/Google/blueline, then in sdm845/Google, sdm845/ and
> >> >> > finally just under hw1.0.
> >> >>
> >> >> Honestly that looks quite compilicated compared to having just one
> >> >> sub-directory. How will ath10k find the directory names (or I vendor and
> >> >> model names) like 'Google' or 'blueline' in this example?
> >> >
> >> > I was thinking about the firmware-name = "sdm845/Google/blueline". But
> >> > this can be really simpler, firmware-name = "blueline" or
> >> > "sdm845/blueline" with no need for fallbacks.
> >>
> >> I have been also thinking about this and I would prefer not to have the
> >> fallbacks. But good if you agree with that.
> >>
> >> IMHO just "sdm845-blueline" would be the most simple. I don't see the
> >> point of having a directory structure when there are not that many
> >> directories really. But this is just cosmetics.
> >
> > It is "not many directories" if we are thinking about the
> > linux-firmware or open devices. But once embedded distros start
> > picking this up for the supported devices, this can quickly become a
> > nuisance.
>
> Ok. Just out of curiosity, any ideas how many devices/products are there
> with wcn3990 who want to use ath10k?
Just for the DT in mainline I can count about 30 devices that have
ath10k WiFi node:
arch/arm64/boot/dts/qcom/msm8998-clamshell.dtsi:&wifi {
arch/arm64/boot/dts/qcom/msm8998-fxtec-pro1.dts:&wifi {
arch/arm64/boot/dts/qcom/msm8998-mtp.dts:&wifi {
arch/arm64/boot/dts/qcom/msm8998-oneplus-common.dtsi:&wifi {
arch/arm64/boot/dts/qcom/msm8998-xiaomi-sagit.dts:&wifi {
arch/arm64/boot/dts/qcom/qcs404-evb.dtsi:&wifi {
arch/arm64/boot/dts/qcom/sc7180-acer-aspire1.dts:&wifi {
arch/arm64/boot/dts/qcom/sc7180-idp.dts:&wifi {
arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi:&wifi {
arch/arm64/boot/dts/qcom/sc7180-trogdor-homestar.dtsi:&wifi {
arch/arm64/boot/dts/qcom/sc7180-trogdor-kingoftown.dts:&wifi {
arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor.dtsi:&wifi {
arch/arm64/boot/dts/qcom/sc7180-trogdor-pazquel360.dtsi:&wifi {
arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom.dtsi:&wifi {
arch/arm64/boot/dts/qcom/sc7180-trogdor-wormdingler.dtsi:&wifi {
arch/arm64/boot/dts/qcom/sc8180x-lenovo-flex-5g.dts:&wifi {
arch/arm64/boot/dts/qcom/sc8180x-primus.dts:&wifi {
arch/arm64/boot/dts/qcom/sdm845-cheza.dtsi:&wifi {
arch/arm64/boot/dts/qcom/sdm845-db845c.dts:&wifi {
arch/arm64/boot/dts/qcom/sdm845-mtp.dts:&wifi {
arch/arm64/boot/dts/qcom/sdm845-oneplus-common.dtsi:&wifi {
arch/arm64/boot/dts/qcom/sdm845-samsung-starqltechn.dts:&wifi {
arch/arm64/boot/dts/qcom/sdm845-shift-axolotl.dts:&wifi {
arch/arm64/boot/dts/qcom/sdm845-xiaomi-beryllium-common.dtsi:&wifi {
arch/arm64/boot/dts/qcom/sdm845-xiaomi-polaris.dts:&wifi {
arch/arm64/boot/dts/qcom/sm6375-sony-xperia-murray-pdx225.dts:&wifi {
arch/arm64/boot/dts/qcom/sm8150-microsoft-surface-duo.dts:&wifi {
arch/arm64/boot/dts/qcom/sm8150-mtp.dts:&wifi {
arch/arm64/boot/dts/qcom/qrb2210-rb1.dts:&wifi {
arch/arm64/boot/dts/qcom/qrb4210-rb2.dts:&wifi {
The list doesn't include e.g. PIxel 2-3-4-5 or some other phones which
are still on their way for the proper mainline support.
--
With best wishes
Dmitry
^ permalink raw reply
* Re: [RESEND v7 34/37] sh: Add dtbs target support.
From: Geert Uytterhoeven @ 2024-04-05 12:45 UTC (permalink / raw)
To: Yoshinori Sato
Cc: linux-sh, Damien Le Moal, Niklas Cassel, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Geert Uytterhoeven,
Michael Turquette, Stephen Boyd, David Airlie, Daniel Vetter,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
Thomas Gleixner, Bjorn Helgaas, Lorenzo Pieralisi,
Krzysztof Wilczyński, Greg Kroah-Hartman, Jiri Slaby,
Magnus Damm, Daniel Lezcano, Rich Felker,
John Paul Adrian Glaubitz, Lee Jones, Helge Deller,
Heiko Stuebner, Shawn Guo, Sebastian Reichel, Chris Morgan,
Linus Walleij, Arnd Bergmann, David Rientjes, Hyeonggon Yoo,
Vlastimil Babka, Baoquan He, Andrew Morton, Guenter Roeck,
Kefeng Wang, Stephen Rothwell, Javier Martinez Canillas, Guo Ren,
Azeem Shaikh, Max Filippov, Jonathan Corbet, Jacky Huang,
Herve Codina, Manikanta Guntupalli, Anup Patel, Biju Das,
Uwe Kleine-König, Sam Ravnborg, Sergey Shtylyov,
Laurent Pinchart, linux-ide, devicetree, linux-kernel,
linux-renesas-soc, linux-clk, dri-devel, linux-pci, linux-serial,
linux-fbdev
In-Reply-To: <f0e220fc338a3dac27f31d7ca871e2ceecad2874.1712207606.git.ysato@users.sourceforge.jp>
On Thu, Apr 4, 2024 at 7:16 AM Yoshinori Sato
<ysato@users.sourceforge.jp> wrote:
> Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp>
My
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
on v6 is still valid.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [RESEND v7 33/37] sh: j2_mimas_v2.dts update
From: Geert Uytterhoeven @ 2024-04-05 12:44 UTC (permalink / raw)
To: Yoshinori Sato
Cc: linux-sh, Damien Le Moal, Niklas Cassel, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Michael Turquette,
Stephen Boyd, David Airlie, Daniel Vetter, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, Thomas Gleixner, Bjorn Helgaas,
Lorenzo Pieralisi, Krzysztof Wilczyński, Greg Kroah-Hartman,
Jiri Slaby, Magnus Damm, Daniel Lezcano, Rich Felker,
John Paul Adrian Glaubitz, Lee Jones, Helge Deller,
Heiko Stuebner, Shawn Guo, Sebastian Reichel, Chris Morgan,
Linus Walleij, Arnd Bergmann, David Rientjes, Hyeonggon Yoo,
Vlastimil Babka, Baoquan He, Andrew Morton, Guenter Roeck,
Kefeng Wang, Stephen Rothwell, Javier Martinez Canillas, Guo Ren,
Azeem Shaikh, Max Filippov, Jonathan Corbet, Jacky Huang,
Herve Codina, Manikanta Guntupalli, Anup Patel, Biju Das,
Uwe Kleine-König, Sam Ravnborg, Sergey Shtylyov,
Laurent Pinchart, linux-ide, devicetree, linux-kernel,
linux-renesas-soc, linux-clk, dri-devel, linux-pci, linux-serial,
linux-fbdev
In-Reply-To: <7cffb0c041744b3c2e324f9908635a912dbb2436.1712207606.git.ysato@users.sourceforge.jp>
Hi Sato-san,
On Thu, Apr 4, 2024 at 7:16 AM Yoshinori Sato
<ysato@users.sourceforge.jp> wrote:
> Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp>
From my comments for v6:
Please enhance the one-line summary, e.g.
sh: j2_mimas_v2: Update CPU compatible value
For the actual changes:
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH 2/3] of: Use scope based kfree() cleanups
From: Rob Herring @ 2024-04-05 12:43 UTC (permalink / raw)
To: Saravana Kannan; +Cc: Jonathan Cameron, devicetree, linux-kernel
In-Reply-To: <CAGETcx_H_vvK9y-51JTcz8F7GDThBwC+t=k2i6r4Nst3H6-TUg@mail.gmail.com>
On Thu, Apr 4, 2024 at 6:16 PM Saravana Kannan <saravanak@google.com> wrote:
>
> On Thu, Apr 4, 2024 at 7:15 AM Rob Herring <robh@kernel.org> wrote:
> >
> > Use the relatively new scope based kfree() cleanup to simplify error
> > handling. Doing so reduces the chances of memory leaks and simplifies
> > error paths by avoiding the need for goto statements.
> >
> > Signed-off-by: Rob Herring <robh@kernel.org>
> > ---
> > drivers/of/base.c | 34 ++++++++--------------------------
> > drivers/of/dynamic.c | 11 ++++-------
> > drivers/of/resolver.c | 35 +++++++++++++----------------------
> > 3 files changed, 25 insertions(+), 55 deletions(-)
> >
> > diff --git a/drivers/of/base.c b/drivers/of/base.c
> > index 8856c67c466a..20603d3c9931 100644
> > --- a/drivers/of/base.c
> > +++ b/drivers/of/base.c
> > @@ -16,6 +16,7 @@
> >
> > #define pr_fmt(fmt) "OF: " fmt
> >
> > +#include <linux/cleanup.h>
> > #include <linux/console.h>
> > #include <linux/ctype.h>
> > #include <linux/cpu.h>
> > @@ -1393,8 +1394,10 @@ int of_parse_phandle_with_args_map(const struct device_node *np,
> > const char *stem_name,
> > int index, struct of_phandle_args *out_args)
> > {
> > - char *cells_name, *map_name = NULL, *mask_name = NULL;
> > - char *pass_name = NULL;
> > + char *cells_name __free(kfree) = kasprintf(GFP_KERNEL, "#%s-cells", stem_name);
> > + char *map_name __free(kfree) = kasprintf(GFP_KERNEL, "%s-map", stem_name);
> > + char *mask_name __free(kfree) = kasprintf(GFP_KERNEL, "%s-map-mask", stem_name);
> > + char *pass_name __free(kfree) = kasprintf(GFP_KERNEL, "%s-map-pass-thru", stem_name);
>
> With the scoped stuff, do these function calls need to be in the same
> line we are defining these variables? If not, I'd rather that the
> calls remain where they were. It feels like a lote to visually parse
> and take in from a readability perspective.
They don't have to be, but if you don't want to get yelled at by the
chief penguin, then yes, they should be together. See the discussions
on adding the scoped iterators. But with the C99 adoption, we can move
the declaration to where the assignment was original.
Rob
^ permalink raw reply
* Re: [PATCH v2 02/18] PCI: endpoint: Introduce pci_epc_map_align()
From: Damien Le Moal @ 2024-04-05 12:43 UTC (permalink / raw)
To: Niklas Cassel
Cc: Kishon Vijay Abraham I, Manivannan Sadhasivam, Lorenzo Pieralisi,
Kishon Vijay Abraham I, Shawn Lin, Krzysztof Wilczyński,
Bjorn Helgaas, Heiko Stuebner, linux-pci, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, devicetree, linux-rockchip,
linux-arm-kernel, Rick Wertenbroek, Wilfred Mallawa
In-Reply-To: <Zg_skLWnl04-pxkn@ryzen>
On 4/5/24 21:20, Niklas Cassel wrote:
> On Thu, Apr 04, 2024 at 11:43:47AM +0900, Damien Le Moal wrote:
>> On 4/3/24 21:33, Kishon Vijay Abraham I wrote:
>>> Hi Damien,
>>>
>>> On 3/30/2024 9:49 AM, Damien Le Moal wrote:
>>>> Some endpoint controllers have requirements on the alignment of the
>>>> controller physical memory address that must be used to map a RC PCI
>>>> address region. For instance, the rockchip endpoint controller uses
>>>> at most the lower 20 bits of a physical memory address region as the
>>>> lower bits of an RC PCI address. For mapping a PCI address region of
>>>> size bytes starting from pci_addr, the exact number of address bits
>>>> used is the number of address bits changing in the address range
>>>> [pci_addr..pci_addr + size - 1].
>>>>
>>>> For this example, this creates the following constraints:
>>>> 1) The offset into the controller physical memory allocated for a
>>>> mapping depends on the mapping size *and* the starting PCI address
>>>> for the mapping.
>>>> 2) A mapping size cannot exceed the controller windows size (1MB) minus
>>>> the offset needed into the allocated physical memory, which can end
>>>> up being a smaller size than the desired mapping size.
>>>>
>>>> Handling these constraints independently of the controller being used in
>>>> a PCI EP function driver is not possible with the current EPC API as
>>>> it only provides the ->align field in struct pci_epc_features.
>>>> Furthermore, this alignment is static and does not depend on a mapping
>>>> pci address and size.
>>>>
>>>> Solve this by introducing the function pci_epc_map_align() and the
>>>> endpoint controller operation ->map_align to allow endpoint function
>>>> drivers to obtain the size and the offset into a controller address
>>>> region that must be used to map an RC PCI address region. The size
>>>> of the physical address region provided by pci_epc_map_align() can then
>>>> be used as the size argument for the function pci_epc_mem_alloc_addr().
>>>> The offset into the allocated controller memory can be used to
>>>> correctly handle data transfers. Of note is that pci_epc_map_align() may
>>>> indicate upon return a mapping size that is smaller (but not 0) than the
>>>> requested PCI address region size. For such case, an endpoint function
>>>> driver must handle data transfers in fragments.
>>>>
>>>> The controller operation ->map_align is optional: controllers that do
>>>> not have any address alignment constraints for mapping a RC PCI address
>>>> region do not need to implement this operation. For such controllers,
>>>> pci_epc_map_align() always returns the mapping size as equal
>>>> to the requested size and an offset equal to 0.
>>>>
>>>> The structure pci_epc_map is introduced to represent a mapping start PCI
>>>> address, size and the size and offset into the controller memory needed
>>>> for mapping the PCI address region.
>>>>
>>>> Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
>>>> ---
>>>> drivers/pci/endpoint/pci-epc-core.c | 66 +++++++++++++++++++++++++++++
>>>> include/linux/pci-epc.h | 33 +++++++++++++++
>>>> 2 files changed, 99 insertions(+)
>>>>
>>>> diff --git a/drivers/pci/endpoint/pci-epc-core.c b/drivers/pci/endpoint/pci-epc-core.c
>>>> index 754afd115bbd..37758ca91d7f 100644
>>>> --- a/drivers/pci/endpoint/pci-epc-core.c
>>>> +++ b/drivers/pci/endpoint/pci-epc-core.c
>>>> @@ -433,6 +433,72 @@ void pci_epc_unmap_addr(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
>>>> }
>>>> EXPORT_SYMBOL_GPL(pci_epc_unmap_addr);
>>>>
>>>> +/**
>>>> + * pci_epc_map_align() - Get the offset into and the size of a controller memory
>>>> + * address region needed to map a RC PCI address region
>>>> + * @epc: the EPC device on which address is allocated
>>>> + * @func_no: the physical endpoint function number in the EPC device
>>>> + * @vfunc_no: the virtual endpoint function number in the physical function
>>>> + * @pci_addr: PCI address to which the physical address should be mapped
>>>> + * @size: the size of the mapping starting from @pci_addr
>>>> + * @map: populate here the actual size and offset into the controller memory
>>>> + * that must be allocated for the mapping
>>>> + *
>>>> + * Invoke the controller map_align operation to obtain the size and the offset
>>>> + * into a controller address region that must be allocated to map @size
>>>> + * bytes of the RC PCI address space starting from @pci_addr.
>>>> + *
>>>> + * The size of the mapping that can be handled by the controller is indicated
>>>> + * using the pci_size field of @map. This size may be smaller than the requested
>>>> + * @size. In such case, the function driver must handle the mapping using
>>>> + * several fragments. The offset into the controller memory for the effective
>>>> + * mapping of the @pci_addr..@pci_addr+@map->pci_size address range is indicated
>>>> + * using the map_ofst field of @map.
>>>> + */
>>>> +int pci_epc_map_align(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
>>>> + u64 pci_addr, size_t size, struct pci_epc_map *map)
>>>> +{
>>>> + const struct pci_epc_features *features;
>>>> + size_t mask;
>>>> + int ret;
>>>> +
>>>> + if (!pci_epc_function_is_valid(epc, func_no, vfunc_no))
>>>> + return -EINVAL;
>>>> +
>>>> + if (!size || !map)
>>>> + return -EINVAL;
>>>> +
>>>> + memset(map, 0, sizeof(*map));
>>>> + map->pci_addr = pci_addr;
>>>> + map->pci_size = size;
>>>> +
>>>> + if (epc->ops->map_align) {
>>>> + mutex_lock(&epc->lock);
>>>> + ret = epc->ops->map_align(epc, func_no, vfunc_no, map);
>>>> + mutex_unlock(&epc->lock);
>>>> + return ret;
>>>> + }
>>>> +
>>>> + /*
>>>> + * Assume a fixed alignment constraint as specified by the controller
>>>> + * features.
>>>> + */
>>>> + features = pci_epc_get_features(epc, func_no, vfunc_no);
>>>> + if (!features || !features->align) {
>>>> + map->map_pci_addr = pci_addr;
>>>> + map->map_size = size;
>>>> + map->map_ofst = 0;
>>>> + }
>>>
>>> The 'align' of pci_epc_features was initially added only to address the
>>> inbound ATU constraints. This is also added as comment in [1]. The PCI
>>> address restrictions (only fixed alignment constraint) were handled by
>>> the host side driver and depends on the connected endpoint device
>>> (atleast it was like that for pci_endpoint_test.c [2]).
>>> So pci-epf-test.c used the 'align' in pci_epc_features only as part of
>>> pci_epf_alloc_space().
>>>
>>> Though I have abused 'align' of pci_epc_features in pci-epf-ntb.c using
>>> it out of pci_epf_alloc_space(), I think we should keep the 'align' of
>>> pci_epc_features only within pci_epf_alloc_space() and controllers with
>>> any PCI address restrictions to implement ->map_align(). This could as
>>> well be done in a phased manner to let controllers implement
>>> ->map_align() and then remove using pci_epc_features in
>>> pci_epc_map_align(). Let me know what you think?
>
> First you say that you want to avoid using epc_features->align inside
> pci_epc_map_align(), and then you say that we could do it in phases,
> and eventually stop using epc_features->align in pci_epc_map_align().
>
> I'm confused... :)
>
> Do you really want pci_epc_map_align() to make use of epc_features->align ?
>
> Don't you mean ep->page_size ?
> (Please read the whole email to see my reasoning.)
>
>
>>
>> Yep, good idea. I will remove the use of "align" as a default alignment
>> constraint. For controllers that have a fixed alignment constraint (not
>> necessarilly epc->features->align), it is trivial to provide a generic helper
>> function that implements the ->map_align method.
>
> We can see that commit:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=2a9a801620efac92885fc9cd53594c0b9aba87a4
>
> Introduced epc_features->align and modified pci_epf_alloc_space() to use it.
>
> From reading the commit, it appears that epc_features->align was intended to
> represent inbound iATU alignment requirement.
>
> For DWC based controllers, the inbound iATU address must be aligned to:
> CX_ATU_MIN_REGION_SIZE.
>
> AFAICT, epc_features->align currently has nothing to do with traffic outbound
> from the EP.
Yes, correct. It is for BARs, not for PCI address alignment constraint.
> For aligning the reads/writes to buffers allocated on the host side,
> we currently have .alignment in the host side driver:
> https://github.com/torvalds/linux/blob/v6.9-rc2/drivers/misc/pci_endpoint_test.c#L966-L1021
>
> Which should be set to the outbound iATU alignment requirement.
>
> For DWC based controllers, the outbound iATU address must be aligned to:
> CX_ATU_MIN_REGION_SIZE.
> Additionally, we have ep->page_size, which defines the smallest outbound unit
> that can be mapped.
> (On DWC based controllers, tis is CX_ATU_MIN_REGION_SIZE.)
>
> ep->page_size is used to specify the outbound alignment for e.g.
> dw_pcie_ep_raise_msi_irq() and dw_pcie_ep_raise_msix_irq():
> https://github.com/torvalds/linux/blob/v6.9-rc2/drivers/pci/controller/dwc/pcie-designware-ep.c#L488
> https://github.com/torvalds/linux/blob/v6.9-rc2/drivers/pci/controller/dwc/pcie-designware-ep.c#L555> which makes sure that we can write to the RC side MSI/MSI-X address
> while satisfying the outbound iATU alignment requirement.
>
> See also:
> https://lore.kernel.org/linux-pci/20240402-pci2_upstream-v3-2-803414bdb430@nxp.com/
>
>
>
> Now I understand that rockchip is the first one that does not have a fixed
> alignment.
> So for that platform, map_align() will be different from ep->page_size.
> (For all DWC based drivers the outbound iATU alignment requirement is
> the same as the page size.)
Yes. So we can have a generic map_align() implementation that all these drivers
can use as there .map_align method. No need to expose page size to the epc/epf
core code.
> However, it would be nice if:
> 1) We could have a default implementation of map_align() that by default uses
> ep->page_size. Platforms that have non-fixed alignment requirements could
> define their own map_align().
See above. The default implementation can be a helper function defined in epc
core that the drivers can use for their .map_align() method.
>
> 2) We fix dw_pcie_ep_raise_msi_irq() and dw_pcie_ep_raise_msix_irq() to use
> the new pci_epc_map_align().
Why ? That is completely internal to the controller driver.
>
> 3) It is getting too complicated with all these...
> epc_features->align, ep->page_size, map_align(), and .alignment in host driver.
> I think that we need to document each of these in Documentation/PCI/endpoint/
test host driver .alignment needs to be nuked. That one is nonsense.
ep->page_size needs to stay internal to the driver. .map_align method is enough
to handle any PCI address mapping constraint and will indicate memory size to
allocate, offset into it etc. And for the BARs alignment, .align feature is not
exactly great as it is not clear, but it is enough I think. So we could just
rename it to be clear. And even maybe remove it from features. I do not see why
an EPF needs to care about it given that epc core funstions are used to setup
the bars.
> 4) It would be nice if we could set page_size correctly for all the PCI device
> and vendor IDs that have defined an .alignment in drivers/misc/pci_endpoint_test.c
> in the correct EPC driver. That way, we should be able to completely remove all
> .alignment specified in drivers/misc/pci_endpoint_test.c.
The host side should be allowed to use any PCI address alignment it wants. So no
alignment communicated at all. It is the EP side that needs to deal with alignment.
> 5) Unfortunately drivers/misc/pci_endpoint_test.c defines a default alignment
> of 4K:
> https://github.com/torvalds/linux/blob/v6.9-rc2/drivers/misc/pci_endpoint_test.c#L968
> https://github.com/torvalds/linux/blob/v6.9-rc2/drivers/misc/pci_endpoint_test.c#L820
>
> It would be nice if we could get rid of this as well. Or perhaps add an option
> to pci_test so that it does not use this 4k alignment, such that we can verify
> that pci_epc_map_align() is actually working.
Exactly. Get rid of any default alignment, add a test parameter to define one so
that we can test different alignment+size combinations.
> In my opinion 4) is the biggest win with this series, because it means that
> we define the alignment in the EPC driver, instead of needing to define it in
> each and every host side driver. But right now, this great improvement is not
> really visible for someone looking quickly at the current series.
Yes. Once in place, we can rework the test driver alignment stuff to make it
optional instead of mandatory because of bad handling on the EP side :)
--
Damien Le Moal
Western Digital Research
^ permalink raw reply
* Re: [PATCH v5 2/2] iio: adc: adding support for PAC193x
From: Conor Dooley @ 2024-04-05 12:41 UTC (permalink / raw)
To: Jonathan Cameron
Cc: marius.cristea, lars, robh+dt, jdelvare, linux, linux-hwmon,
krzysztof.kozlowski+dt, conor+dt, linux-iio, devicetree,
linux-kernel
In-Reply-To: <20240224191559.40d233db@jic23-huawei>
[-- Attachment #1: Type: text/plain, Size: 2742 bytes --]
On Sat, Feb 24, 2024 at 07:15:59PM +0000, Jonathan Cameron wrote:
> On Thu, 22 Feb 2024 18:42:06 +0200
> <marius.cristea@microchip.com> wrote:
>
> > From: Marius Cristea <marius.cristea@microchip.com>
> >
> > This is the iio driver for Microchip
> > PAC193X series of Power Monitor with Accumulator chip family.
> >
> > Signed-off-by: Marius Cristea <marius.cristea@microchip.com>
> So I had a few comments on this, but nothing that can't be cleaned up later.
> + I'll fix the thing the bots didn't like on the bindings.
>
> Series applied to the togreg branch of iio.git and pushed out
> as testing for 0-day to take a look at it.
I tested this out on v6.9-rc2 and prompted a backtrace when collectd
started running:
------------[ cut here ]------------
UBSAN: array-index-out-of-bounds in /home/conor/stuff/linux/drivers/iio/adc/pac1934.c:857:25
index 7 is out of range for type 'u32 [4]'
CPU: 1 PID: 179 Comm: iiod Not tainted 6.9.0-rc2-dirty #1
Hardware name: Microchip PolarFire-SoC Icicle Kit (DT)
Call Trace:
[<ffffffff80006bba>] dump_backtrace+0x28/0x30
[<ffffffff80bd67d8>] show_stack+0x38/0x44
[<ffffffff80be7820>] dump_stack_lvl+0x6e/0x9a
[<ffffffff80be7864>] dump_stack+0x18/0x20
[<ffffffff80be1452>] ubsan_epilogue+0x10/0x46
[<ffffffff80615358>] __ubsan_handle_out_of_bounds+0x6a/0x78
[<ffffffff80981f3a>] pac1934_read_raw+0x20c/0x34c
[<ffffffff80977c4c>] iio_read_channel_info+0x5c/0xbe
[<ffffffff8073516e>] dev_attr_show+0x1c/0x4a
[<ffffffff80387292>] sysfs_kf_seq_show+0x80/0xcc
[<ffffffff80385b12>] kernfs_seq_show+0x3c/0x4a
[<ffffffff8031e3d8>] seq_read_iter+0x136/0x2e4
[<ffffffff80385cde>] kernfs_fop_read_iter+0x38/0x16a
[<ffffffff802e904a>] vfs_read+0x1be/0x2ba
[<ffffffff802e9c48>] ksys_read+0x64/0xd2
[<ffffffff802e9cd6>] __riscv_sys_read+0x20/0x28
[<ffffffff80be838a>] do_trap_ecall_u+0xee/0x204
[<ffffffff80bf57d0>] ret_from_exception+0x0/0x64
---[ end trace ]---
The device itself only has 4 channels, but in sysfs there are "fake"
channels for the average voltages and currents too. UBSAN points at:
case PAC1934_VSENSE_AVG_4_ADDR:
*val = PAC1934_MAX_VSENSE_RSHIFTED_BY_16B;
if (chan->scan_type.sign == 'u')
*val2 = info->shunts[channel];
else
*val2 = info->shunts[channel] >> 1;
return IIO_VAL_FRACTIONAL;
And info->shunts is only valid for the 4 real channels, so I guess the
averaged channels probably need to do a [channel - 4] or similar. I
dunno if that relation between averaged channels and their "real"
counterparts is fixed or if different pac devices need different values,
but on my system here that would work.
I do quite like that UBSAN points out the line in question :)
Cheers,
Conor.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH v3 5/9] drm: xlnx: zynqmp_dpsub: Minimize usage of global flag
From: Tomi Valkeinen @ 2024-04-05 12:42 UTC (permalink / raw)
To: Anatoliy Klymenko
Cc: dri-devel, linux-arm-kernel, linux-kernel, devicetree,
linux-media, Laurent Pinchart, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Daniel Vetter, Michal Simek,
Andrzej Hajda, Neil Armstrong, Robert Foss, Jonas Karlman,
Jernej Skrabec, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Mauro Carvalho Chehab
In-Reply-To: <20240321-dp-live-fmt-v3-5-d5090d796b7e@amd.com>
On 21/03/2024 22:43, Anatoliy Klymenko wrote:
> Avoid usage of global zynqmp_dpsub.dma_enabled flag in DPSUB layer
> context. This flag signals whether the driver runs in DRM CRTC or DRM
> bridge mode, assuming that all display layers share the same live or
> non-live mode of operation. Using per-layer mode instead of global flag
> will simplify future support of the hybrid scenario.
>
> Remove redundant checks in DMA request/release contexts as
> zynqmp_disp_layer.info is well-defined for all layer types, including the
> correct number of DMA channels required for each particular layer.
>
> Signed-off-by: Anatoliy Klymenko <anatoliy.klymenko@amd.com>
> ---
> drivers/gpu/drm/xlnx/zynqmp_disp.c | 12 +++---------
> 1 file changed, 3 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/xlnx/zynqmp_disp.c b/drivers/gpu/drm/xlnx/zynqmp_disp.c
> index abdc3971b193..0c2b3f4bffa6 100644
> --- a/drivers/gpu/drm/xlnx/zynqmp_disp.c
> +++ b/drivers/gpu/drm/xlnx/zynqmp_disp.c
> @@ -980,7 +980,7 @@ void zynqmp_disp_layer_disable(struct zynqmp_disp_layer *layer)
> {
> unsigned int i;
>
> - if (layer->disp->dpsub->dma_enabled) {
> + if (layer->mode == ZYNQMP_DPSUB_LAYER_NONLIVE) {
> for (i = 0; i < layer->drm_fmt->num_planes; i++)
> dmaengine_terminate_sync(layer->dmas[i].chan);
> }
> @@ -1006,7 +1006,7 @@ void zynqmp_disp_layer_set_format(struct zynqmp_disp_layer *layer,
>
> zynqmp_disp_avbuf_set_format(layer->disp, layer, layer->disp_fmt);
>
> - if (!layer->disp->dpsub->dma_enabled)
> + if (layer->mode == ZYNQMP_DPSUB_LAYER_LIVE)
> return;
>
> /*
> @@ -1044,7 +1044,7 @@ int zynqmp_disp_layer_update(struct zynqmp_disp_layer *layer,
> const struct drm_format_info *info = layer->drm_fmt;
> unsigned int i;
>
> - if (!layer->disp->dpsub->dma_enabled)
> + if (layer->mode == ZYNQMP_DPSUB_LAYER_LIVE)
> return 0;
>
> for (i = 0; i < info->num_planes; i++) {
> @@ -1094,9 +1094,6 @@ static void zynqmp_disp_layer_release_dma(struct zynqmp_disp *disp,
> {
> unsigned int i;
>
> - if (!layer->info || !disp->dpsub->dma_enabled)
> - return;
> -
> for (i = 0; i < layer->info->num_channels; i++) {
> struct zynqmp_disp_layer_dma *dma = &layer->dmas[i];
>
> @@ -1137,9 +1134,6 @@ static int zynqmp_disp_layer_request_dma(struct zynqmp_disp *disp,
> unsigned int i;
> int ret;
>
> - if (!disp->dpsub->dma_enabled)
> - return 0;
> -
> for (i = 0; i < layer->info->num_channels; i++) {
> struct zynqmp_disp_layer_dma *dma = &layer->dmas[i];
> char dma_channel_name[16];
>
Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Tomi
^ permalink raw reply
* Re: [PATCH RFC v2 0/4] wifi: ath10k: support board-specific firmware overrides
From: Kalle Valo @ 2024-04-05 12:41 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Jeff Johnson, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Bjorn Andersson, Konrad Dybcio, ath10k, linux-wireless, netdev,
devicetree, linux-arm-msm, Krzysztof Kozlowski
In-Reply-To: <CAA8EJppASEmj6-Jt7OCABAeqr8umSgXaDDha9nn2nRafuZ-Gvw@mail.gmail.com>
Dmitry Baryshkov <dmitry.baryshkov@linaro.org> writes:
> On Fri, 5 Apr 2024 at 15:01, Kalle Valo <kvalo@kernel.org> wrote:
>
>>
>> Dmitry Baryshkov <dmitry.baryshkov@linaro.org> writes:
>>
>> > On Fri, 8 Mar 2024 at 17:19, Kalle Valo <kvalo@kernel.org> wrote:
>> >>
>> >> Dmitry Baryshkov <dmitry.baryshkov@linaro.org> writes:
>> >>
>> >> >> To be on the safe side using 'qcom-rb1' makes sense but on the other
>> >> >> hand that means we need to update linux-firmware (basically add a new
>> >> >> symlink) everytime a new product is added. But are there going to be
>> >> >> that many new ath10k based products?
>> >> >>
>> >> >> Using 'qcm2290' is easier because for a new product then there only
>> >> >> needs to be a change in DTS and no need to change anything
>> >> >> linux-firmware. But here the risk is that if there's actually two
>> >> >> different ath10k firmware branches for 'qcm2290'. If that ever happens
>> >> >> (I hope not) I guess we could solve that by adding new 'qcm2290-foo'
>> >> >> directory?
>> >> >>
>> >> >> But I don't really know, thoughts?
>> >> >
>> >> > After some thought, I'd suggest to follow approach taken by the rest
>> >> > of qcom firmware:
>> >>
>> >> Can you provide pointers to those cases?
>> >
>> > https://gitlab.com/kernel-firmware/linux-firmware/-/tree/main/qcom/sc8280xp/LENOVO/21BX
>> >
>> >>
>> >> > put a default (accepted by non-secured hardware) firmware to SoC dir
>> >> > and then put a vendor-specific firmware into subdir. If any of such
>> >> > vendors appear, we might even implement structural fallback: first
>> >> > look into sdm845/Google/blueline, then in sdm845/Google, sdm845/ and
>> >> > finally just under hw1.0.
>> >>
>> >> Honestly that looks quite compilicated compared to having just one
>> >> sub-directory. How will ath10k find the directory names (or I vendor and
>> >> model names) like 'Google' or 'blueline' in this example?
>> >
>> > I was thinking about the firmware-name = "sdm845/Google/blueline". But
>> > this can be really simpler, firmware-name = "blueline" or
>> > "sdm845/blueline" with no need for fallbacks.
>>
>> I have been also thinking about this and I would prefer not to have the
>> fallbacks. But good if you agree with that.
>>
>> IMHO just "sdm845-blueline" would be the most simple. I don't see the
>> point of having a directory structure when there are not that many
>> directories really. But this is just cosmetics.
>
> It is "not many directories" if we are thinking about the
> linux-firmware or open devices. But once embedded distros start
> picking this up for the supported devices, this can quickly become a
> nuisance.
Ok. Just out of curiosity, any ideas how many devices/products are there
with wcn3990 who want to use ath10k?
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [PATCH v3 4/9] drm: xlnx: zynqmp_dpsub: Anounce supported input formats
From: Tomi Valkeinen @ 2024-04-05 12:39 UTC (permalink / raw)
To: Anatoliy Klymenko
Cc: dri-devel, linux-arm-kernel, linux-kernel, devicetree,
linux-media, Laurent Pinchart, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Daniel Vetter, Michal Simek,
Andrzej Hajda, Neil Armstrong, Robert Foss, Jonas Karlman,
Jernej Skrabec, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Mauro Carvalho Chehab
In-Reply-To: <20240321-dp-live-fmt-v3-4-d5090d796b7e@amd.com>
On 21/03/2024 22:43, Anatoliy Klymenko wrote:
> DPSUB in bridge mode supports multiple input media bus formats.
>
> Announce the list of supported input media bus formats via
> drm_bridge.atomic_get_input_bus_fmts callback.
> Introduce a set of live input formats, supported by DPSUB.
> Rename zynqmp_disp_layer_drm_formats() to zynqmp_disp_layer_formats() to
> reflect semantics for both live and non-live layer format lists.
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Anatoliy Klymenko <anatoliy.klymenko@amd.com>
> ---
> drivers/gpu/drm/xlnx/zynqmp_disp.c | 76 +++++++++++++++++++++++++++++++++-----
> drivers/gpu/drm/xlnx/zynqmp_disp.h | 4 +-
> drivers/gpu/drm/xlnx/zynqmp_dp.c | 31 ++++++++++++++++
> drivers/gpu/drm/xlnx/zynqmp_kms.c | 2 +-
> 4 files changed, 101 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/xlnx/zynqmp_disp.c b/drivers/gpu/drm/xlnx/zynqmp_disp.c
> index e6d26ef60e89..abdc3971b193 100644
> --- a/drivers/gpu/drm/xlnx/zynqmp_disp.c
> +++ b/drivers/gpu/drm/xlnx/zynqmp_disp.c
> @@ -18,6 +18,7 @@
> #include <linux/dma/xilinx_dpdma.h>
> #include <linux/dma-mapping.h>
> #include <linux/dmaengine.h>
> +#include <linux/media-bus-format.h>
> #include <linux/module.h>
> #include <linux/of.h>
> #include <linux/platform_device.h>
> @@ -77,12 +78,14 @@ enum zynqmp_dpsub_layer_mode {
> /**
> * struct zynqmp_disp_format - Display subsystem format information
> * @drm_fmt: DRM format (4CC)
> + * @bus_fmt: Media bus format
> * @buf_fmt: AV buffer format
> * @swap: Flag to swap R & B for RGB formats, and U & V for YUV formats
> * @sf: Scaling factors for color components
> */
> struct zynqmp_disp_format {
> u32 drm_fmt;
> + u32 bus_fmt;
> u32 buf_fmt;
> bool swap;
> const u32 *sf;
> @@ -182,6 +185,12 @@ static const u32 scaling_factors_565[] = {
> ZYNQMP_DISP_AV_BUF_5BIT_SF,
> };
>
> +static const u32 scaling_factors_666[] = {
> + ZYNQMP_DISP_AV_BUF_6BIT_SF,
> + ZYNQMP_DISP_AV_BUF_6BIT_SF,
> + ZYNQMP_DISP_AV_BUF_6BIT_SF,
> +};
> +
> static const u32 scaling_factors_888[] = {
> ZYNQMP_DISP_AV_BUF_8BIT_SF,
> ZYNQMP_DISP_AV_BUF_8BIT_SF,
> @@ -364,6 +373,41 @@ static const struct zynqmp_disp_format avbuf_gfx_fmts[] = {
> },
> };
>
> +/* List of live video layer formats */
> +static const struct zynqmp_disp_format avbuf_live_fmts[] = {
> + {
> + .drm_fmt = DRM_FORMAT_RGB565,
> + .bus_fmt = MEDIA_BUS_FMT_RGB666_1X18,
> + .buf_fmt = ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_BPC_6 |
> + ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_FMT_RGB,
> + .sf = scaling_factors_666,
> + }, {
> + .drm_fmt = DRM_FORMAT_RGB888,
> + .bus_fmt = MEDIA_BUS_FMT_RGB888_1X24,
> + .buf_fmt = ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_BPC_8 |
> + ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_FMT_RGB,
> + .sf = scaling_factors_888,
> + }, {
> + .drm_fmt = DRM_FORMAT_YUV422,
> + .bus_fmt = MEDIA_BUS_FMT_UYVY8_1X16,
> + .buf_fmt = ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_BPC_8 |
> + ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_FMT_YUV422,
> + .sf = scaling_factors_888,
> + }, {
> + .drm_fmt = DRM_FORMAT_YUV444,
> + .bus_fmt = MEDIA_BUS_FMT_VUY8_1X24,
> + .buf_fmt = ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_BPC_8 |
> + ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_FMT_YUV444,
> + .sf = scaling_factors_888,
> + }, {
> + .drm_fmt = DRM_FORMAT_P210,
> + .bus_fmt = MEDIA_BUS_FMT_UYVY10_1X20,
> + .buf_fmt = ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_BPC_10 |
> + ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_FMT_YUV422,
> + .sf = scaling_factors_101010,
> + },
> +};
> +
> static u32 zynqmp_disp_avbuf_read(struct zynqmp_disp *disp, int reg)
> {
> return readl(disp->avbuf.base + reg);
> @@ -883,16 +927,17 @@ zynqmp_disp_layer_find_format(struct zynqmp_disp_layer *layer,
> }
>
> /**
> - * zynqmp_disp_layer_drm_formats - Return the DRM formats supported by the layer
> + * zynqmp_disp_layer_formats - Return DRM or media bus formats supported by
> + * the layer
> * @layer: The layer
> * @num_formats: Pointer to the returned number of formats
> *
> - * Return: A newly allocated u32 array that stores all the DRM formats
> + * Return: A newly allocated u32 array that stores all DRM or media bus formats
> * supported by the layer. The number of formats in the array is returned
> * through the num_formats argument.
> */
> -u32 *zynqmp_disp_layer_drm_formats(struct zynqmp_disp_layer *layer,
> - unsigned int *num_formats)
> +u32 *zynqmp_disp_layer_formats(struct zynqmp_disp_layer *layer,
> + unsigned int *num_formats)
> {
> unsigned int i;
> u32 *formats;
> @@ -903,7 +948,9 @@ u32 *zynqmp_disp_layer_drm_formats(struct zynqmp_disp_layer *layer,
> return NULL;
>
> for (i = 0; i < layer->info->num_formats; ++i)
> - formats[i] = layer->info->formats[i].drm_fmt;
> + formats[i] = layer->mode == ZYNQMP_DPSUB_LAYER_NONLIVE
> + ? layer->info->formats[i].drm_fmt
> + : layer->info->formats[i].bus_fmt;
I find this quite confusing. Depending on the layer mode, you return
different format types. I think it's quite easy to use this kind of
function the wrong way.
Why not just make two separate functions?
Tomi
> *num_formats = layer->info->num_formats;
> return formats;
> @@ -1131,6 +1178,11 @@ static int zynqmp_disp_create_layers(struct zynqmp_disp *disp)
> .num_channels = 1,
> },
> };
> + static const struct zynqmp_disp_layer_info live_layer_info = {
> + .formats = avbuf_live_fmts,
> + .num_formats = ARRAY_SIZE(avbuf_live_fmts),
> + .num_channels = 0,
> + };
>
> unsigned int i;
> int ret;
> @@ -1140,12 +1192,18 @@ static int zynqmp_disp_create_layers(struct zynqmp_disp *disp)
>
> layer->id = i;
> layer->disp = disp;
> - layer->info = &layer_info[i];
> - /* For now assume dpsub works in either live or non-live mode for both layers.
> +
> + /*
> + * For now assume dpsub works in either live or non-live mode for both layers.
> * Hybrid mode is not supported yet.
> */
> - layer->mode = disp->dpsub->dma_enabled ? ZYNQMP_DPSUB_LAYER_NONLIVE
> - : ZYNQMP_DPSUB_LAYER_LIVE;
> + if (disp->dpsub->dma_enabled) {
> + layer->mode = ZYNQMP_DPSUB_LAYER_NONLIVE;
> + layer->info = &layer_info[i];
> + } else {
> + layer->mode = ZYNQMP_DPSUB_LAYER_LIVE;
> + layer->info = &live_layer_info;
> + }
>
> ret = zynqmp_disp_layer_request_dma(disp, layer);
> if (ret)
> diff --git a/drivers/gpu/drm/xlnx/zynqmp_disp.h b/drivers/gpu/drm/xlnx/zynqmp_disp.h
> index 9b8b202224d9..88c285a12e23 100644
> --- a/drivers/gpu/drm/xlnx/zynqmp_disp.h
> +++ b/drivers/gpu/drm/xlnx/zynqmp_disp.h
> @@ -50,8 +50,8 @@ int zynqmp_disp_setup_clock(struct zynqmp_disp *disp,
> void zynqmp_disp_blend_set_global_alpha(struct zynqmp_disp *disp,
> bool enable, u32 alpha);
>
> -u32 *zynqmp_disp_layer_drm_formats(struct zynqmp_disp_layer *layer,
> - unsigned int *num_formats);
> +u32 *zynqmp_disp_layer_formats(struct zynqmp_disp_layer *layer,
> + unsigned int *num_formats);
> void zynqmp_disp_layer_enable(struct zynqmp_disp_layer *layer);
> void zynqmp_disp_layer_disable(struct zynqmp_disp_layer *layer);
> void zynqmp_disp_layer_set_format(struct zynqmp_disp_layer *layer,
> diff --git a/drivers/gpu/drm/xlnx/zynqmp_dp.c b/drivers/gpu/drm/xlnx/zynqmp_dp.c
> index 4faafdd76798..e3b9eb3d9273 100644
> --- a/drivers/gpu/drm/xlnx/zynqmp_dp.c
> +++ b/drivers/gpu/drm/xlnx/zynqmp_dp.c
> @@ -22,6 +22,7 @@
> #include <linux/delay.h>
> #include <linux/device.h>
> #include <linux/io.h>
> +#include <linux/media-bus-format.h>
> #include <linux/module.h>
> #include <linux/platform_device.h>
> #include <linux/pm_runtime.h>
> @@ -1577,6 +1578,35 @@ static const struct drm_edid *zynqmp_dp_bridge_edid_read(struct drm_bridge *brid
> return drm_edid_read_ddc(connector, &dp->aux.ddc);
> }
>
> +static u32 *zynqmp_dp_bridge_default_bus_fmts(unsigned int *num_input_fmts)
> +{
> + u32 *formats = kzalloc(sizeof(*formats), GFP_KERNEL);
> +
> + if (formats)
> + *formats = MEDIA_BUS_FMT_FIXED;
> + *num_input_fmts = !!formats;
> +
> + return formats;
> +}
> +
> +static u32 *
> +zynqmp_dp_bridge_get_input_bus_fmts(struct drm_bridge *bridge,
> + struct drm_bridge_state *bridge_state,
> + struct drm_crtc_state *crtc_state,
> + struct drm_connector_state *conn_state,
> + u32 output_fmt,
> + unsigned int *num_input_fmts)
> +{
> + struct zynqmp_dp *dp = bridge_to_dp(bridge);
> + struct zynqmp_disp_layer *layer;
> +
> + layer = zynqmp_dp_disp_connected_live_layer(dp);
> + if (layer)
> + return zynqmp_disp_layer_formats(layer, num_input_fmts);
> + else
> + return zynqmp_dp_bridge_default_bus_fmts(num_input_fmts);
> +}
> +
> static const struct drm_bridge_funcs zynqmp_dp_bridge_funcs = {
> .attach = zynqmp_dp_bridge_attach,
> .detach = zynqmp_dp_bridge_detach,
> @@ -1589,6 +1619,7 @@ static const struct drm_bridge_funcs zynqmp_dp_bridge_funcs = {
> .atomic_check = zynqmp_dp_bridge_atomic_check,
> .detect = zynqmp_dp_bridge_detect,
> .edid_read = zynqmp_dp_bridge_edid_read,
> + .atomic_get_input_bus_fmts = zynqmp_dp_bridge_get_input_bus_fmts,
> };
>
> /* -----------------------------------------------------------------------------
> diff --git a/drivers/gpu/drm/xlnx/zynqmp_kms.c b/drivers/gpu/drm/xlnx/zynqmp_kms.c
> index 43bf416b33d5..bf9fba01df0e 100644
> --- a/drivers/gpu/drm/xlnx/zynqmp_kms.c
> +++ b/drivers/gpu/drm/xlnx/zynqmp_kms.c
> @@ -152,7 +152,7 @@ static int zynqmp_dpsub_create_planes(struct zynqmp_dpsub *dpsub)
> unsigned int num_formats;
> u32 *formats;
>
> - formats = zynqmp_disp_layer_drm_formats(layer, &num_formats);
> + formats = zynqmp_disp_layer_formats(layer, &num_formats);
> if (!formats)
> return -ENOMEM;
>
>
^ permalink raw reply
* Re: [PATCH RFC v2 0/4] wifi: ath10k: support board-specific firmware overrides
From: Dmitry Baryshkov @ 2024-04-05 12:34 UTC (permalink / raw)
To: Kalle Valo
Cc: Jeff Johnson, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Bjorn Andersson, Konrad Dybcio, ath10k, linux-wireless, netdev,
devicetree, linux-arm-msm, Krzysztof Kozlowski
In-Reply-To: <871q7k3tnq.fsf@kernel.org>
On Fri, 5 Apr 2024 at 15:01, Kalle Valo <kvalo@kernel.org> wrote:
>
> Dmitry Baryshkov <dmitry.baryshkov@linaro.org> writes:
>
> > On Fri, 8 Mar 2024 at 17:19, Kalle Valo <kvalo@kernel.org> wrote:
> >>
> >> Dmitry Baryshkov <dmitry.baryshkov@linaro.org> writes:
> >>
> >> >> To be on the safe side using 'qcom-rb1' makes sense but on the other
> >> >> hand that means we need to update linux-firmware (basically add a new
> >> >> symlink) everytime a new product is added. But are there going to be
> >> >> that many new ath10k based products?
> >> >>
> >> >> Using 'qcm2290' is easier because for a new product then there only
> >> >> needs to be a change in DTS and no need to change anything
> >> >> linux-firmware. But here the risk is that if there's actually two
> >> >> different ath10k firmware branches for 'qcm2290'. If that ever happens
> >> >> (I hope not) I guess we could solve that by adding new 'qcm2290-foo'
> >> >> directory?
> >> >>
> >> >> But I don't really know, thoughts?
> >> >
> >> > After some thought, I'd suggest to follow approach taken by the rest
> >> > of qcom firmware:
> >>
> >> Can you provide pointers to those cases?
> >
> > https://gitlab.com/kernel-firmware/linux-firmware/-/tree/main/qcom/sc8280xp/LENOVO/21BX
> >
> >>
> >> > put a default (accepted by non-secured hardware) firmware to SoC dir
> >> > and then put a vendor-specific firmware into subdir. If any of such
> >> > vendors appear, we might even implement structural fallback: first
> >> > look into sdm845/Google/blueline, then in sdm845/Google, sdm845/ and
> >> > finally just under hw1.0.
> >>
> >> Honestly that looks quite compilicated compared to having just one
> >> sub-directory. How will ath10k find the directory names (or I vendor and
> >> model names) like 'Google' or 'blueline' in this example?
> >
> > I was thinking about the firmware-name = "sdm845/Google/blueline". But
> > this can be really simpler, firmware-name = "blueline" or
> > "sdm845/blueline" with no need for fallbacks.
>
> I have been also thinking about this and I would prefer not to have the
> fallbacks. But good if you agree with that.
>
> IMHO just "sdm845-blueline" would be the most simple. I don't see the
> point of having a directory structure when there are not that many
> directories really. But this is just cosmetics.
It is "not many directories" if we are thinking about the
linux-firmware or open devices. But once embedded distros start
picking this up for the supported devices, this can quickly become a
nuisance. We have been there for Qualcomm DSP firmware and we ended up
adopting the SoC/vendor/device structure, because otherwise it becomes
a bedlam.
> > My point is that the firmware-name provides the possibility to handle
> > that in different ways.
>
> Very good, thanks.
Thanks for your suggestions and for picking the patches.
Bjorn, could you please pick up the DT patches now?
--
With best wishes
Dmitry
^ permalink raw reply
* Re: [RESEND v7 09/37] dt-binding: Add compatible SH7750 SoC
From: Geert Uytterhoeven @ 2024-04-05 12:31 UTC (permalink / raw)
To: Yoshinori Sato
Cc: linux-sh, Damien Le Moal, Niklas Cassel, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Geert Uytterhoeven,
Michael Turquette, Stephen Boyd, David Airlie, Daniel Vetter,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
Thomas Gleixner, Bjorn Helgaas, Lorenzo Pieralisi,
Krzysztof Wilczyński, Greg Kroah-Hartman, Jiri Slaby,
Magnus Damm, Daniel Lezcano, Rich Felker,
John Paul Adrian Glaubitz, Lee Jones, Helge Deller,
Heiko Stuebner, Shawn Guo, Sebastian Reichel, Chris Morgan,
Linus Walleij, Arnd Bergmann, David Rientjes, Hyeonggon Yoo,
Vlastimil Babka, Baoquan He, Andrew Morton, Guenter Roeck,
Kefeng Wang, Stephen Rothwell, Javier Martinez Canillas, Guo Ren,
Azeem Shaikh, Max Filippov, Jonathan Corbet, Jacky Huang,
Herve Codina, Manikanta Guntupalli, Anup Patel, Biju Das,
Uwe Kleine-König, Sam Ravnborg, Sergey Shtylyov,
Laurent Pinchart, linux-ide, devicetree, linux-kernel,
linux-renesas-soc, linux-clk, dri-devel, linux-pci, linux-serial,
linux-fbdev
In-Reply-To: <4ac65d0f311e890c1ca92bf057c70954ec7ac351.1712207606.git.ysato@users.sourceforge.jp>
On Thu, Apr 4, 2024 at 7:15 AM Yoshinori Sato
<ysato@users.sourceforge.jp> wrote:
> Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* [PATCH v2 6/6] firmware: imx: add i.MX95 MISC driver
From: Peng Fan (OSS) @ 2024-04-05 12:39 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
Sudeep Holla, Cristian Marussi
Cc: Peng Fan, devicetree, imx, linux-arm-kernel, linux-kernel
In-Reply-To: <20240405-imx95-bbm-misc-v2-v2-0-9fc9186856c2@nxp.com>
From: Peng Fan <peng.fan@nxp.com>
The i.MX95 System manager exports SCMI MISC protocol for linux to do
various settings, such as set board gpio expander as wakeup source.
The driver is to add the support.
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
drivers/firmware/imx/Makefile | 1 +
drivers/firmware/imx/sm-misc.c | 92 +++++++++++++++++++++++++++++++++++++++++
include/linux/firmware/imx/sm.h | 33 +++++++++++++++
3 files changed, 126 insertions(+)
diff --git a/drivers/firmware/imx/Makefile b/drivers/firmware/imx/Makefile
index fb20e22074e1..cb9c361d9b81 100644
--- a/drivers/firmware/imx/Makefile
+++ b/drivers/firmware/imx/Makefile
@@ -2,3 +2,4 @@
obj-$(CONFIG_IMX_DSP) += imx-dsp.o
obj-$(CONFIG_IMX_SCU) += imx-scu.o misc.o imx-scu-irq.o rm.o imx-scu-soc.o
obj-${CONFIG_IMX_SCMI_BBM_EXT} += sm-bbm.o
+obj-${CONFIG_IMX_SCMI_MISC_EXT} += sm-misc.o
diff --git a/drivers/firmware/imx/sm-misc.c b/drivers/firmware/imx/sm-misc.c
new file mode 100644
index 000000000000..a5609de426f6
--- /dev/null
+++ b/drivers/firmware/imx/sm-misc.c
@@ -0,0 +1,92 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright 2024 NXP.
+ */
+
+#include <linux/firmware/imx/sm.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/scmi_protocol.h>
+#include <linux/scmi_imx_protocol.h>
+
+static const struct scmi_imx_misc_proto_ops *imx_misc_ctrl_ops;
+static struct scmi_protocol_handle *ph;
+struct notifier_block scmi_imx_misc_ctrl_nb;
+
+int scmi_imx_misc_ctrl_set(u32 id, u32 val)
+{
+ if (!ph)
+ return -EPROBE_DEFER;
+
+ return imx_misc_ctrl_ops->misc_ctrl_set(ph, id, 1, &val);
+};
+EXPORT_SYMBOL(scmi_imx_misc_ctrl_set);
+
+int scmi_imx_misc_ctrl_get(u32 id, u32 *num, u32 *val)
+{
+ if (!ph)
+ return -EPROBE_DEFER;
+
+ return imx_misc_ctrl_ops->misc_ctrl_get(ph, id, num, val);
+}
+EXPORT_SYMBOL(scmi_imx_misc_ctrl_get);
+
+static int scmi_imx_misc_ctrl_notifier(struct notifier_block *nb,
+ unsigned long event, void *data)
+{
+ return 0;
+}
+
+static int scmi_imx_misc_ctrl_probe(struct scmi_device *sdev)
+{
+ const struct scmi_handle *handle = sdev->handle;
+ struct device_node *np = sdev->dev.of_node;
+ u32 src_id, evt_id, wu_num;
+ int ret, i;
+
+ if (!handle)
+ return -ENODEV;
+
+ imx_misc_ctrl_ops = handle->devm_protocol_get(sdev, SCMI_PROTOCOL_IMX_MISC, &ph);
+ if (IS_ERR(imx_misc_ctrl_ops))
+ return PTR_ERR(imx_misc_ctrl_ops);
+
+ scmi_imx_misc_ctrl_nb.notifier_call = &scmi_imx_misc_ctrl_notifier;
+ wu_num = of_property_count_u32_elems(np, "wakeup-sources");
+ if (wu_num % 2) {
+ dev_err(&sdev->dev, "Invalid wakeup-sources\n");
+ return -EINVAL;
+ }
+
+ for (i = 0; i < wu_num; i += 2) {
+ WARN_ON(of_property_read_u32_index(np, "wakeup-sources", i, &src_id));
+ WARN_ON(of_property_read_u32_index(np, "wakeup-sources", i + 1, &evt_id));
+ ret = handle->notify_ops->devm_event_notifier_register(sdev, SCMI_PROTOCOL_IMX_MISC,
+ evt_id,
+ &src_id,
+ &scmi_imx_misc_ctrl_nb);
+ if (ret)
+ dev_err(&sdev->dev, "Failed to register scmi misc event: %d\n", src_id);
+ }
+
+ return 0;
+
+}
+
+static const struct scmi_device_id scmi_id_table[] = {
+ { SCMI_PROTOCOL_IMX_MISC, "imx-misc-ctrl" },
+ { },
+};
+MODULE_DEVICE_TABLE(scmi, scmi_id_table);
+
+static struct scmi_driver scmi_imx_misc_ctrl_driver = {
+ .name = "scmi-imx-misc-ctrl",
+ .probe = scmi_imx_misc_ctrl_probe,
+ .id_table = scmi_id_table,
+};
+module_scmi_driver(scmi_imx_misc_ctrl_driver);
+
+MODULE_AUTHOR("Peng Fan <peng.fan@nxp.com>");
+MODULE_DESCRIPTION("IMX SM MISC driver");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/firmware/imx/sm.h b/include/linux/firmware/imx/sm.h
new file mode 100644
index 000000000000..daad4bdf7d1c
--- /dev/null
+++ b/include/linux/firmware/imx/sm.h
@@ -0,0 +1,33 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Copyright 2024 NXP
+ */
+
+#ifndef _SCMI_IMX_H
+#define _SCMI_IMX_H
+
+#include <linux/bitfield.h>
+#include <linux/types.h>
+
+#define SCMI_IMX_CTRL_PDM_CLK_SEL 0 /* AON PDM clock sel */
+#define SCMI_IMX_CTRL_MQS1_SETTINGS 1 /* AON MQS settings */
+#define SCMI_IMX_CTRL_SAI1_MCLK 2 /* AON SAI1 MCLK */
+#define SCMI_IMX_CTRL_SAI3_MCLK 3 /* WAKE SAI3 MCLK */
+#define SCMI_IMX_CTRL_SAI4_MCLK 4 /* WAKE SAI4 MCLK */
+#define SCMI_IMX_CTRL_SAI5_MCLK 5 /* WAKE SAI5 MCLK */
+
+#if IS_ENABLED(CONFIG_IMX_SCMI_MISC_EXT)
+int scmi_imx_misc_ctrl_get(u32 id, u32 *num, u32 *val);
+int scmi_imx_misc_ctrl_set(u32 id, u32 val);
+#else
+static inline int scmi_imx_misc_ctrl_get(u32 id, u32 *num, u32 *val)
+{
+ return -EOPNOTSUPP;
+}
+
+static inline int scmi_imx_misc_ctrl_set(u32 id, u32 val);
+{
+ return -EOPNOTSUPP;
+}
+#endif
+#endif
--
2.37.1
^ permalink raw reply related
* [PATCH v2 5/6] firmware: imx: support BBM module
From: Peng Fan (OSS) @ 2024-04-05 12:39 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
Sudeep Holla, Cristian Marussi
Cc: Peng Fan, devicetree, imx, linux-arm-kernel, linux-kernel
In-Reply-To: <20240405-imx95-bbm-misc-v2-v2-0-9fc9186856c2@nxp.com>
From: Peng Fan <peng.fan@nxp.com>
The BBM module provides RTC and BUTTON feature. To i.MX95, this module
is managed by System Manager. Linux could use i.MX SCMI BBM Extension
protocol to use RTC and BUTTON feature.
This driver is to use SCMI interface to get/set RTC, enable pwrkey.
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
drivers/firmware/imx/Makefile | 1 +
drivers/firmware/imx/sm-bbm.c | 317 ++++++++++++++++++++++++++++++++++++++++++
2 files changed, 318 insertions(+)
diff --git a/drivers/firmware/imx/Makefile b/drivers/firmware/imx/Makefile
index 8f9f04a513a8..fb20e22074e1 100644
--- a/drivers/firmware/imx/Makefile
+++ b/drivers/firmware/imx/Makefile
@@ -1,3 +1,4 @@
# SPDX-License-Identifier: GPL-2.0
obj-$(CONFIG_IMX_DSP) += imx-dsp.o
obj-$(CONFIG_IMX_SCU) += imx-scu.o misc.o imx-scu-irq.o rm.o imx-scu-soc.o
+obj-${CONFIG_IMX_SCMI_BBM_EXT} += sm-bbm.o
diff --git a/drivers/firmware/imx/sm-bbm.c b/drivers/firmware/imx/sm-bbm.c
new file mode 100644
index 000000000000..fcb2ae8490c8
--- /dev/null
+++ b/drivers/firmware/imx/sm-bbm.c
@@ -0,0 +1,317 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright 2024 NXP.
+ */
+
+#include <linux/input.h>
+#include <linux/jiffies.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/rtc.h>
+#include <linux/scmi_protocol.h>
+#include <linux/scmi_imx_protocol.h>
+#include <linux/suspend.h>
+
+#define DEBOUNCE_TIME 30
+#define REPEAT_INTERVAL 60
+
+struct scmi_imx_bbm {
+ struct rtc_device *rtc_dev;
+ struct scmi_protocol_handle *ph;
+ const struct scmi_imx_bbm_proto_ops *ops;
+ struct notifier_block nb;
+ int keycode;
+ int keystate; /* 1:pressed */
+ bool suspended;
+ struct delayed_work check_work;
+ struct input_dev *input;
+};
+
+static int scmi_imx_bbm_read_time(struct device *dev, struct rtc_time *tm)
+{
+ struct scmi_imx_bbm *bbnsm = dev_get_drvdata(dev);
+ struct scmi_protocol_handle *ph = bbnsm->ph;
+ u64 val;
+ int ret;
+
+ ret = bbnsm->ops->rtc_time_get(ph, 0, &val);
+ if (ret)
+ dev_err(dev, "%s: %d\n", __func__, ret);
+
+ rtc_time64_to_tm(val, tm);
+
+ return 0;
+}
+
+static int scmi_imx_bbm_set_time(struct device *dev, struct rtc_time *tm)
+{
+ struct scmi_imx_bbm *bbnsm = dev_get_drvdata(dev);
+ struct scmi_protocol_handle *ph = bbnsm->ph;
+ u64 val;
+ int ret;
+
+ val = rtc_tm_to_time64(tm);
+
+ ret = bbnsm->ops->rtc_time_set(ph, 0, val);
+ if (ret)
+ dev_err(dev, "%s: %d\n", __func__, ret);
+
+ return 0;
+}
+
+static int scmi_imx_bbm_alarm_irq_enable(struct device *dev, unsigned int enable)
+{
+ return 0;
+}
+
+static int scmi_imx_bbm_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
+{
+ struct scmi_imx_bbm *bbnsm = dev_get_drvdata(dev);
+ struct scmi_protocol_handle *ph = bbnsm->ph;
+ struct rtc_time *alrm_tm = &alrm->time;
+ u64 val;
+ int ret;
+
+ val = rtc_tm_to_time64(alrm_tm);
+
+ ret = bbnsm->ops->rtc_alarm_set(ph, 0, val);
+ if (ret)
+ dev_err(dev, "%s: %d\n", __func__, ret);
+
+ return 0;
+}
+
+static const struct rtc_class_ops smci_imx_bbm_rtc_ops = {
+ .read_time = scmi_imx_bbm_read_time,
+ .set_time = scmi_imx_bbm_set_time,
+ .set_alarm = scmi_imx_bbm_set_alarm,
+ .alarm_irq_enable = scmi_imx_bbm_alarm_irq_enable,
+};
+
+static void scmi_imx_bbm_pwrkey_check_for_events(struct work_struct *work)
+{
+ struct scmi_imx_bbm *bbnsm = container_of(work, struct scmi_imx_bbm, check_work.work);
+ struct scmi_protocol_handle *ph = bbnsm->ph;
+ struct input_dev *input = bbnsm->input;
+ u32 state = 0;
+ int ret;
+
+ ret = bbnsm->ops->button_get(ph, &state);
+ if (ret) {
+ pr_err("%s: %d\n", __func__, ret);
+ return;
+ }
+
+ pr_debug("%s: state: %d, keystate %d\n", __func__, state, bbnsm->keystate);
+
+ /* only report new event if status changed */
+ if (state ^ bbnsm->keystate) {
+ bbnsm->keystate = state;
+ input_event(input, EV_KEY, bbnsm->keycode, state);
+ input_sync(input);
+ pm_relax(bbnsm->input->dev.parent);
+ pr_debug("EV_KEY: %x\n", bbnsm->keycode);
+ }
+
+ /* repeat check if pressed long */
+ if (state)
+ schedule_delayed_work(&bbnsm->check_work, msecs_to_jiffies(REPEAT_INTERVAL));
+}
+
+static int scmi_imx_bbm_pwrkey_event(struct scmi_imx_bbm *bbnsm)
+{
+ struct input_dev *input = bbnsm->input;
+
+ schedule_delayed_work(&bbnsm->check_work, msecs_to_jiffies(DEBOUNCE_TIME));
+
+ /*
+ * Directly report key event after resume to make no key press
+ * event is missed.
+ */
+ if (bbnsm->suspended) {
+ bbnsm->keystate = 1;
+ input_event(input, EV_KEY, bbnsm->keycode, 1);
+ input_sync(input);
+ }
+
+ return 0;
+}
+
+static void scmi_imx_bbm_pwrkey_act(void *pdata)
+{
+ struct scmi_imx_bbm *bbnsm = pdata;
+
+ cancel_delayed_work_sync(&bbnsm->check_work);
+}
+
+static int scmi_imx_bbm_notifier(struct notifier_block *nb, unsigned long event, void *data)
+{
+ struct scmi_imx_bbm *bbnsm = container_of(nb, struct scmi_imx_bbm, nb);
+ struct scmi_imx_bbm_notif_report *r = data;
+
+ if (r->is_rtc)
+ rtc_update_irq(bbnsm->rtc_dev, 1, RTC_AF | RTC_IRQF);
+ if (r->is_button) {
+ pr_debug("BBM Button Power key pressed\n");
+ scmi_imx_bbm_pwrkey_event(bbnsm);
+ }
+
+ return 0;
+}
+
+static int scmi_imx_bbm_pwrkey_init(struct scmi_device *sdev)
+{
+ const struct scmi_handle *handle = sdev->handle;
+ struct device *dev = &sdev->dev;
+ struct scmi_imx_bbm *bbnsm = dev_get_drvdata(dev);
+ struct input_dev *input;
+ int ret;
+
+ if (device_property_read_u32(dev, "linux,code", &bbnsm->keycode)) {
+ bbnsm->keycode = KEY_POWER;
+ dev_warn(dev, "key code is not specified, using default KEY_POWER\n");
+ }
+
+ INIT_DELAYED_WORK(&bbnsm->check_work, scmi_imx_bbm_pwrkey_check_for_events);
+
+ input = devm_input_allocate_device(dev);
+ if (!input) {
+ dev_err(dev, "failed to allocate the input device for SCMI IMX BBM\n");
+ return -ENOMEM;
+ }
+
+ input->name = dev_name(dev);
+ input->phys = "bbnsm-pwrkey/input0";
+ input->id.bustype = BUS_HOST;
+
+ input_set_capability(input, EV_KEY, bbnsm->keycode);
+
+ ret = devm_add_action_or_reset(dev, scmi_imx_bbm_pwrkey_act, bbnsm);
+ if (ret) {
+ dev_err(dev, "failed to register remove action\n");
+ return ret;
+ }
+
+ bbnsm->input = input;
+
+ ret = handle->notify_ops->devm_event_notifier_register(sdev, SCMI_PROTOCOL_IMX_BBM,
+ SCMI_EVENT_IMX_BBM_BUTTON,
+ NULL, &bbnsm->nb);
+
+ if (ret)
+ dev_err(dev, "Failed to register BBM Button Events %d:", ret);
+
+ ret = input_register_device(input);
+ if (ret) {
+ dev_err(dev, "failed to register input device\n");
+ return ret;
+ }
+
+ return 0;
+}
+
+static int scmi_imx_bbm_rtc_init(struct scmi_device *sdev)
+{
+ const struct scmi_handle *handle = sdev->handle;
+ struct device *dev = &sdev->dev;
+ struct scmi_imx_bbm *bbnsm = dev_get_drvdata(dev);
+ int ret;
+
+ bbnsm->rtc_dev = devm_rtc_allocate_device(dev);
+ if (IS_ERR(bbnsm->rtc_dev))
+ return PTR_ERR(bbnsm->rtc_dev);
+
+ bbnsm->rtc_dev->ops = &smci_imx_bbm_rtc_ops;
+ bbnsm->rtc_dev->range_min = 0;
+ bbnsm->rtc_dev->range_max = U32_MAX;
+
+ ret = devm_rtc_register_device(bbnsm->rtc_dev);
+ if (ret)
+ return ret;
+
+ bbnsm->nb.notifier_call = &scmi_imx_bbm_notifier;
+ return handle->notify_ops->devm_event_notifier_register(sdev, SCMI_PROTOCOL_IMX_BBM,
+ SCMI_EVENT_IMX_BBM_RTC,
+ NULL, &bbnsm->nb);
+}
+
+static int scmi_imx_bbm_probe(struct scmi_device *sdev)
+{
+ const struct scmi_handle *handle = sdev->handle;
+ struct device *dev = &sdev->dev;
+ struct scmi_protocol_handle *ph;
+ struct scmi_imx_bbm *bbnsm;
+ int ret;
+
+ if (!handle)
+ return -ENODEV;
+
+ bbnsm = devm_kzalloc(dev, sizeof(struct scmi_imx_bbm), GFP_KERNEL);
+ if (!bbnsm)
+ return -ENOMEM;
+
+ bbnsm->ops = handle->devm_protocol_get(sdev, SCMI_PROTOCOL_IMX_BBM, &ph);
+ if (IS_ERR(bbnsm->ops))
+ return PTR_ERR(bbnsm->ops);
+
+ bbnsm->ph = ph;
+
+ device_init_wakeup(dev, true);
+
+ dev_set_drvdata(dev, bbnsm);
+
+ ret = scmi_imx_bbm_rtc_init(sdev);
+ if (ret) {
+ dev_err(dev, "rtc init failed: %d\n", ret);
+ return ret;
+ }
+
+ ret = scmi_imx_bbm_pwrkey_init(sdev);
+ if (ret) {
+ dev_err(dev, "pwr init failed: %d\n", ret);
+ return ret;
+ }
+
+ return 0;
+}
+
+static int __maybe_unused scmi_imx_bbm_suspend(struct device *dev)
+{
+ struct scmi_imx_bbm *bbnsm = dev_get_drvdata(dev);
+
+ bbnsm->suspended = true;
+
+ return 0;
+}
+
+static int __maybe_unused scmi_imx_bbm_resume(struct device *dev)
+{
+ struct scmi_imx_bbm *bbnsm = dev_get_drvdata(dev);
+
+ bbnsm->suspended = false;
+
+ return 0;
+}
+
+static SIMPLE_DEV_PM_OPS(scmi_imx_bbm_pm_ops, scmi_imx_bbm_suspend, scmi_imx_bbm_resume);
+
+static const struct scmi_device_id scmi_id_table[] = {
+ { SCMI_PROTOCOL_IMX_BBM, "imx-bbm" },
+ { },
+};
+MODULE_DEVICE_TABLE(scmi, scmi_id_table);
+
+static struct scmi_driver scmi_imx_bbm_driver = {
+ .driver = {
+ .pm = &scmi_imx_bbm_pm_ops,
+ },
+ .name = "scmi-imx-bbm",
+ .probe = scmi_imx_bbm_probe,
+ .id_table = scmi_id_table,
+};
+module_scmi_driver(scmi_imx_bbm_driver);
+
+MODULE_AUTHOR("Peng Fan <peng.fan@nxp.com>");
+MODULE_DESCRIPTION("IMX SM BBM driver");
+MODULE_LICENSE("GPL");
--
2.37.1
^ permalink raw reply related
* [PATCH v2 4/6] firmware: arm_scmi: add initial support for i.MX MISC protocol
From: Peng Fan (OSS) @ 2024-04-05 12:39 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
Sudeep Holla, Cristian Marussi
Cc: Peng Fan, devicetree, imx, linux-arm-kernel, linux-kernel
In-Reply-To: <20240405-imx95-bbm-misc-v2-v2-0-9fc9186856c2@nxp.com>
From: Peng Fan <peng.fan@nxp.com>
The i.MX MISC protocol is for misc settings, such as gpio expander
wakeup.
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
drivers/firmware/arm_scmi/Kconfig | 10 ++
drivers/firmware/arm_scmi/Makefile | 1 +
drivers/firmware/arm_scmi/imx-sm-misc.c | 305 ++++++++++++++++++++++++++++++++
include/linux/scmi_imx_protocol.h | 17 ++
4 files changed, 333 insertions(+)
diff --git a/drivers/firmware/arm_scmi/Kconfig b/drivers/firmware/arm_scmi/Kconfig
index 56d11c9d9f47..bfeae92f6420 100644
--- a/drivers/firmware/arm_scmi/Kconfig
+++ b/drivers/firmware/arm_scmi/Kconfig
@@ -191,3 +191,13 @@ config IMX_SCMI_BBM_EXT
and BUTTON.
This driver can also be built as a module.
+
+config IMX_SCMI_MISC_EXT
+ tristate "i.MX SCMI MISC EXTENSION"
+ depends on ARM_SCMI_PROTOCOL || (COMPILE_TEST && OF)
+ default y if ARCH_MXC
+ help
+ This enables i.MX System MISC control logic such as gpio expander
+ wakeup
+
+ This driver can also be built as a module.
diff --git a/drivers/firmware/arm_scmi/Makefile b/drivers/firmware/arm_scmi/Makefile
index 327687acf857..a23fde721222 100644
--- a/drivers/firmware/arm_scmi/Makefile
+++ b/drivers/firmware/arm_scmi/Makefile
@@ -12,6 +12,7 @@ scmi-transport-$(CONFIG_ARM_SCMI_TRANSPORT_VIRTIO) += virtio.o
scmi-transport-$(CONFIG_ARM_SCMI_TRANSPORT_OPTEE) += optee.o
scmi-protocols-y = base.o clock.o perf.o power.o reset.o sensors.o system.o voltage.o powercap.o
scmi-protocols-$(CONFIG_IMX_SCMI_BBM_EXT) += imx-sm-bbm.o
+scmi-protocols-$(CONFIG_IMX_SCMI_MISC_EXT) += imx-sm-misc.o
scmi-module-objs := $(scmi-driver-y) $(scmi-protocols-y) $(scmi-transport-y)
obj-$(CONFIG_ARM_SCMI_PROTOCOL) += scmi-core.o
diff --git a/drivers/firmware/arm_scmi/imx-sm-misc.c b/drivers/firmware/arm_scmi/imx-sm-misc.c
new file mode 100644
index 000000000000..1b0ec2281518
--- /dev/null
+++ b/drivers/firmware/arm_scmi/imx-sm-misc.c
@@ -0,0 +1,305 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * System control and Management Interface (SCMI) NXP MISC Protocol
+ *
+ * Copyright 2024 NXP
+ */
+
+#define pr_fmt(fmt) "SCMI Notifications MISC - " fmt
+
+#include <linux/bits.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/scmi_protocol.h>
+#include <linux/scmi_imx_protocol.h>
+
+#include "protocols.h"
+#include "notify.h"
+
+#define SCMI_PROTOCOL_SUPPORTED_VERSION 0x10000
+
+enum scmi_imx_misc_protocol_cmd {
+ SCMI_IMX_MISC_CTRL_SET = 0x3,
+ SCMI_IMX_MISC_CTRL_GET = 0x4,
+ SCMI_IMX_MISC_CTRL_NOTIFY = 0x8,
+};
+
+struct scmi_imx_misc_info {
+ u32 version;
+ u32 nr_dev_ctrl;
+ u32 nr_brd_ctrl;
+ u32 nr_reason;
+};
+
+struct scmi_msg_imx_misc_protocol_attributes {
+ __le32 attributes;
+};
+
+#define GET_BRD_CTRLS_NR(x) le32_get_bits((x), GENMASK(31, 24))
+#define GET_REASONS_NR(x) le32_get_bits((x), GENMASK(23, 16))
+#define GET_DEV_CTRLS_NR(x) le32_get_bits((x), GENMASK(15, 0))
+#define BRD_CTRL_START_ID BIT(15)
+
+struct scmi_imx_misc_ctrl_set_in {
+ __le32 id;
+ __le32 num;
+ __le32 value[MISC_MAX_VAL];
+};
+
+struct scmi_imx_misc_ctrl_notify_in {
+ __le32 ctrl_id;
+ __le32 flags;
+};
+
+struct scmi_imx_misc_ctrl_notify_payld {
+ __le32 ctrl_id;
+ __le32 flags;
+};
+
+struct scmi_imx_misc_ctrl_get_out {
+ __le32 num;
+ __le32 *val;
+};
+
+static int scmi_imx_misc_attributes_get(const struct scmi_protocol_handle *ph,
+ struct scmi_imx_misc_info *mi)
+{
+ int ret;
+ struct scmi_xfer *t;
+ struct scmi_msg_imx_misc_protocol_attributes *attr;
+
+ ret = ph->xops->xfer_get_init(ph, PROTOCOL_ATTRIBUTES, 0,
+ sizeof(*attr), &t);
+ if (ret)
+ return ret;
+
+ attr = t->rx.buf;
+
+ ret = ph->xops->do_xfer(ph, t);
+ if (!ret) {
+ mi->nr_dev_ctrl = GET_DEV_CTRLS_NR(attr->attributes);
+ mi->nr_brd_ctrl = GET_BRD_CTRLS_NR(attr->attributes);
+ mi->nr_reason = GET_REASONS_NR(attr->attributes);
+ dev_info(ph->dev, "i.MX MISC NUM DEV CTRL: %d, NUM BRD CTRL: %d,NUM Reason: %d\n",
+ mi->nr_dev_ctrl, mi->nr_brd_ctrl, mi->nr_reason);
+ }
+
+ ph->xops->xfer_put(ph, t);
+
+ return ret;
+}
+
+static int scmi_imx_misc_ctrl_validate_id(const struct scmi_protocol_handle *ph,
+ u32 ctrl_id)
+{
+ struct scmi_imx_misc_info *mi = ph->get_priv(ph);
+
+ if ((ctrl_id < BRD_CTRL_START_ID) && (ctrl_id > mi->nr_dev_ctrl))
+ return -EINVAL;
+ if (ctrl_id >= BRD_CTRL_START_ID + mi->nr_brd_ctrl)
+ return -EINVAL;
+
+ return 0;
+}
+
+static int scmi_imx_misc_ctrl_notify(const struct scmi_protocol_handle *ph,
+ u32 ctrl_id, u32 flags)
+{
+ struct scmi_imx_misc_ctrl_notify_in *in;
+ struct scmi_xfer *t;
+ int ret;
+
+ ret = scmi_imx_misc_ctrl_validate_id(ph, ctrl_id);
+ if (ret)
+ return ret;
+
+ ret = ph->xops->xfer_get_init(ph, SCMI_IMX_MISC_CTRL_NOTIFY,
+ sizeof(*in), 0, &t);
+ if (ret)
+ return ret;
+
+ in = t->tx.buf;
+ in->ctrl_id = cpu_to_le32(ctrl_id);
+ in->flags = cpu_to_le32(flags);
+
+ ret = ph->xops->do_xfer(ph, t);
+
+ ph->xops->xfer_put(ph, t);
+
+ return ret;
+}
+
+static int
+scmi_imx_misc_ctrl_set_notify_enabled(const struct scmi_protocol_handle *ph,
+ u8 evt_id, u32 src_id, bool enable)
+{
+ int ret;
+
+ ret = scmi_imx_misc_ctrl_notify(ph, src_id, enable ? evt_id : 0);
+ if (ret)
+ dev_err(ph->dev, "FAIL_ENABLED - evt[%X] src[%d] - ret:%d\n",
+ evt_id, src_id, ret);
+
+ return ret;
+}
+
+static int scmi_imx_misc_ctrl_get_num_sources(const struct scmi_protocol_handle *ph)
+{
+ return GENMASK(15, 0);
+}
+
+static void *
+scmi_imx_misc_ctrl_fill_custom_report(const struct scmi_protocol_handle *ph,
+ u8 evt_id, ktime_t timestamp,
+ const void *payld, size_t payld_sz,
+ void *report, u32 *src_id)
+{
+ const struct scmi_imx_misc_ctrl_notify_payld *p = payld;
+ struct scmi_imx_misc_ctrl_notify_report *r = report;
+
+ if (sizeof(*p) != payld_sz)
+ return NULL;
+
+ r->timestamp = timestamp;
+ r->ctrl_id = p->ctrl_id;
+ r->flags = p->flags;
+ *src_id = r->ctrl_id;
+ dev_dbg(ph->dev, "%s: ctrl_id: %d flags: %d\n", __func__,
+ r->ctrl_id, r->flags);
+
+ return r;
+}
+
+static const struct scmi_event_ops scmi_imx_misc_event_ops = {
+ .get_num_sources = scmi_imx_misc_ctrl_get_num_sources,
+ .set_notify_enabled = scmi_imx_misc_ctrl_set_notify_enabled,
+ .fill_custom_report = scmi_imx_misc_ctrl_fill_custom_report,
+};
+
+static const struct scmi_event scmi_imx_misc_events[] = {
+ {
+ .id = SCMI_EVENT_IMX_MISC_CONTROL_DISABLED,
+ .max_payld_sz = sizeof(struct scmi_imx_misc_ctrl_notify_payld),
+ .max_report_sz = sizeof(struct scmi_imx_misc_ctrl_notify_report),
+ },
+ {
+ .id = SCMI_EVENT_IMX_MISC_CONTROL_FALLING_EDGE,
+ .max_payld_sz = sizeof(struct scmi_imx_misc_ctrl_notify_payld),
+ .max_report_sz = sizeof(struct scmi_imx_misc_ctrl_notify_report),
+ },
+ {
+ .id = SCMI_EVENT_IMX_MISC_CONTROL_RISING_EDGE,
+ .max_payld_sz = sizeof(struct scmi_imx_misc_ctrl_notify_payld),
+ .max_report_sz = sizeof(struct scmi_imx_misc_ctrl_notify_report),
+ }
+};
+
+static struct scmi_protocol_events scmi_imx_misc_protocol_events = {
+ .queue_sz = SCMI_PROTO_QUEUE_SZ,
+ .ops = &scmi_imx_misc_event_ops,
+ .evts = scmi_imx_misc_events,
+ .num_events = ARRAY_SIZE(scmi_imx_misc_events),
+};
+
+static int scmi_imx_misc_protocol_init(const struct scmi_protocol_handle *ph)
+{
+ struct scmi_imx_misc_info *minfo;
+ u32 version;
+ int ret;
+
+ ret = ph->xops->version_get(ph, &version);
+ if (ret)
+ return ret;
+
+ dev_info(ph->dev, "NXP SM MISC Version %d.%d\n",
+ PROTOCOL_REV_MAJOR(version), PROTOCOL_REV_MINOR(version));
+
+ minfo = devm_kzalloc(ph->dev, sizeof(*minfo), GFP_KERNEL);
+ if (!minfo)
+ return -ENOMEM;
+
+ ret = scmi_imx_misc_attributes_get(ph, minfo);
+ if (ret)
+ return ret;
+
+ return ph->set_priv(ph, minfo, version);
+}
+
+static int scmi_imx_misc_ctrl_get(const struct scmi_protocol_handle *ph,
+ u32 ctrl_id, u32 *num, u32 *val)
+{
+ struct scmi_imx_misc_ctrl_get_out *out;
+ struct scmi_xfer *t;
+ int ret, i;
+
+ ret = scmi_imx_misc_ctrl_validate_id(ph, ctrl_id);
+ if (ret)
+ return ret;
+
+ ret = ph->xops->xfer_get_init(ph, SCMI_IMX_MISC_CTRL_GET, sizeof(u32),
+ 0, &t);
+ if (ret)
+ return ret;
+
+ put_unaligned_le32(ctrl_id, t->tx.buf);
+ ret = ph->xops->do_xfer(ph, t);
+ if (!ret) {
+ out = t->rx.buf;
+ *num = le32_to_cpu(out->num);
+ for (i = 0; i < *num && i < MISC_MAX_VAL; i++)
+ val[i] = le32_to_cpu(out->val[i]);
+ }
+
+ ph->xops->xfer_put(ph, t);
+
+ return ret;
+}
+
+static int scmi_imx_misc_ctrl_set(const struct scmi_protocol_handle *ph,
+ u32 ctrl_id, u32 num, u32 *val)
+{
+ struct scmi_imx_misc_ctrl_set_in *in;
+ struct scmi_xfer *t;
+ int ret, i;
+
+ ret = scmi_imx_misc_ctrl_validate_id(ph, ctrl_id);
+ if (ret)
+ return ret;
+
+ if (num > MISC_MAX_VAL)
+ return -EINVAL;
+
+ ret = ph->xops->xfer_get_init(ph, SCMI_IMX_MISC_CTRL_SET, sizeof(*in),
+ 0, &t);
+ if (ret)
+ return ret;
+
+ in = t->tx.buf;
+ in->id = cpu_to_le32(ctrl_id);
+ in->num = cpu_to_le32(num);
+ for (i = 0; i < num; i++)
+ in->value[i] = cpu_to_le32(val[i]);
+
+ ret = ph->xops->do_xfer(ph, t);
+
+ ph->xops->xfer_put(ph, t);
+
+ return ret;
+}
+
+static const struct scmi_imx_misc_proto_ops scmi_imx_misc_proto_ops = {
+ .misc_ctrl_set = scmi_imx_misc_ctrl_set,
+ .misc_ctrl_get = scmi_imx_misc_ctrl_get,
+};
+
+static const struct scmi_protocol scmi_imx_misc = {
+ .id = SCMI_PROTOCOL_IMX_MISC,
+ .owner = THIS_MODULE,
+ .instance_init = &scmi_imx_misc_protocol_init,
+ .ops = &scmi_imx_misc_proto_ops,
+ .events = &scmi_imx_misc_protocol_events,
+ .supported_version = SCMI_PROTOCOL_SUPPORTED_VERSION,
+};
+module_scmi_protocol(scmi_imx_misc);
diff --git a/include/linux/scmi_imx_protocol.h b/include/linux/scmi_imx_protocol.h
index 90ce011a4429..a69bd4a20f0f 100644
--- a/include/linux/scmi_imx_protocol.h
+++ b/include/linux/scmi_imx_protocol.h
@@ -13,8 +13,14 @@
#include <linux/notifier.h>
#include <linux/types.h>
+#define SCMI_PAYLOAD_LEN 100
+
+#define SCMI_ARRAY(X, Y) ((SCMI_PAYLOAD_LEN - (X)) / sizeof(Y))
+#define MISC_MAX_VAL SCMI_ARRAY(8, uint32_t)
+
enum scmi_nxp_protocol {
SCMI_PROTOCOL_IMX_BBM = 0x81,
+ SCMI_PROTOCOL_IMX_MISC = 0x84,
};
struct scmi_imx_bbm_proto_ops {
@@ -42,4 +48,15 @@ struct scmi_imx_bbm_notif_report {
unsigned int rtc_id;
unsigned int rtc_evt;
};
+
+struct scmi_imx_misc_ctrl_notify_report {
+ ktime_t timestamp;
+ unsigned int ctrl_id;
+ unsigned int flags;
+};
+
+struct scmi_imx_misc_proto_ops {
+ int (*misc_ctrl_set)(const struct scmi_protocol_handle *ph, u32 id, u32 num, u32 *val);
+ int (*misc_ctrl_get)(const struct scmi_protocol_handle *ph, u32 id, u32 *num, u32 *val);
+};
#endif
--
2.37.1
^ permalink raw reply related
* [PATCH v2 3/6] firmware: arm_scmi: add initial support for i.MX BBM protocol
From: Peng Fan (OSS) @ 2024-04-05 12:39 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
Sudeep Holla, Cristian Marussi
Cc: Peng Fan, devicetree, imx, linux-arm-kernel, linux-kernel
In-Reply-To: <20240405-imx95-bbm-misc-v2-v2-0-9fc9186856c2@nxp.com>
From: Peng Fan <peng.fan@nxp.com>
The i.MX BBM protocol is for managing i.MX BBM module which provides
RTC and BUTTON feature.
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
drivers/firmware/arm_scmi/Kconfig | 10 +
drivers/firmware/arm_scmi/Makefile | 1 +
drivers/firmware/arm_scmi/imx-sm-bbm.c | 378 +++++++++++++++++++++++++++++++++
include/linux/scmi_imx_protocol.h | 45 ++++
4 files changed, 434 insertions(+)
diff --git a/drivers/firmware/arm_scmi/Kconfig b/drivers/firmware/arm_scmi/Kconfig
index aa5842be19b2..56d11c9d9f47 100644
--- a/drivers/firmware/arm_scmi/Kconfig
+++ b/drivers/firmware/arm_scmi/Kconfig
@@ -181,3 +181,13 @@ config ARM_SCMI_POWER_CONTROL
early shutdown/reboot SCMI requests.
endmenu
+
+config IMX_SCMI_BBM_EXT
+ tristate "i.MX SCMI BBM EXTENSION"
+ depends on ARM_SCMI_PROTOCOL || (COMPILE_TEST && OF)
+ default y if ARCH_MXC
+ help
+ This enables i.MX System BBM control logic which supports RTC
+ and BUTTON.
+
+ This driver can also be built as a module.
diff --git a/drivers/firmware/arm_scmi/Makefile b/drivers/firmware/arm_scmi/Makefile
index a7bc4796519c..327687acf857 100644
--- a/drivers/firmware/arm_scmi/Makefile
+++ b/drivers/firmware/arm_scmi/Makefile
@@ -11,6 +11,7 @@ scmi-transport-$(CONFIG_ARM_SCMI_HAVE_MSG) += msg.o
scmi-transport-$(CONFIG_ARM_SCMI_TRANSPORT_VIRTIO) += virtio.o
scmi-transport-$(CONFIG_ARM_SCMI_TRANSPORT_OPTEE) += optee.o
scmi-protocols-y = base.o clock.o perf.o power.o reset.o sensors.o system.o voltage.o powercap.o
+scmi-protocols-$(CONFIG_IMX_SCMI_BBM_EXT) += imx-sm-bbm.o
scmi-module-objs := $(scmi-driver-y) $(scmi-protocols-y) $(scmi-transport-y)
obj-$(CONFIG_ARM_SCMI_PROTOCOL) += scmi-core.o
diff --git a/drivers/firmware/arm_scmi/imx-sm-bbm.c b/drivers/firmware/arm_scmi/imx-sm-bbm.c
new file mode 100644
index 000000000000..92c0aedf65cc
--- /dev/null
+++ b/drivers/firmware/arm_scmi/imx-sm-bbm.c
@@ -0,0 +1,378 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * System Control and Management Interface (SCMI) NXP BBM Protocol
+ *
+ * Copyright 2024 NXP
+ */
+
+#define pr_fmt(fmt) "SCMI Notifications BBM - " fmt
+
+#include <linux/bits.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/scmi_protocol.h>
+#include <linux/scmi_imx_protocol.h>
+
+#include "protocols.h"
+#include "notify.h"
+
+#define SCMI_PROTOCOL_SUPPORTED_VERSION 0x10000
+
+enum scmi_imx_bbm_protocol_cmd {
+ IMX_BBM_GPR_SET = 0x3,
+ IMX_BBM_GPR_GET = 0x4,
+ IMX_BBM_RTC_ATTRIBUTES = 0x5,
+ IMX_BBM_RTC_TIME_SET = 0x6,
+ IMX_BBM_RTC_TIME_GET = 0x7,
+ IMX_BBM_RTC_ALARM_SET = 0x8,
+ IMX_BBM_BUTTON_GET = 0x9,
+ IMX_BBM_RTC_NOTIFY = 0xA,
+ IMX_BBM_BUTTON_NOTIFY = 0xB,
+};
+
+#define GET_RTCS_NR(x) le32_get_bits((x), GENMASK(23, 16))
+#define GET_GPRS_NR(x) le32_get_bits((x), GENMASK(15, 0))
+
+#define SCMI_IMX_BBM_NOTIFY_RTC_UPDATED BIT(2)
+#define SCMI_IMX_BBM_NOTIFY_RTC_ROLLOVER BIT(1)
+#define SCMI_IMX_BBM_NOTIFY_RTC_ALARM BIT(0)
+
+#define SCMI_IMX_BBM_RTC_ALARM_ENABLE_FLAG BIT(0)
+
+#define SCMI_IMX_BBM_NOTIFY_RTC_FLAG \
+ (SCMI_IMX_BBM_NOTIFY_RTC_UPDATED | SCMI_IMX_BBM_NOTIFY_RTC_ROLLOVER | \
+ SCMI_IMX_BBM_NOTIFY_RTC_ALARM)
+
+#define SCMI_IMX_BBM_EVENT_RTC_MASK GENMASK(31, 24)
+
+struct scmi_imx_bbm_info {
+ u32 version;
+ int nr_rtc;
+ int nr_gpr;
+};
+
+struct scmi_msg_imx_bbm_protocol_attributes {
+ __le32 attributes;
+};
+
+struct scmi_imx_bbm_set_time {
+ __le32 id;
+ __le32 flags;
+ __le32 value_low;
+ __le32 value_high;
+};
+
+struct scmi_imx_bbm_get_time {
+ __le32 id;
+ __le32 flags;
+};
+
+struct scmi_imx_bbm_alarm_time {
+ __le32 id;
+ __le32 flags;
+ __le32 value_low;
+ __le32 value_high;
+};
+
+struct scmi_msg_imx_bbm_rtc_notify {
+ __le32 rtc_id;
+ __le32 flags;
+};
+
+struct scmi_msg_imx_bbm_button_notify {
+ __le32 flags;
+};
+
+struct scmi_imx_bbm_notify_payld {
+ __le32 flags;
+};
+
+static int scmi_imx_bbm_attributes_get(const struct scmi_protocol_handle *ph,
+ struct scmi_imx_bbm_info *pi)
+{
+ int ret;
+ struct scmi_xfer *t;
+ struct scmi_msg_imx_bbm_protocol_attributes *attr;
+
+ ret = ph->xops->xfer_get_init(ph, PROTOCOL_ATTRIBUTES, 0, sizeof(*attr), &t);
+ if (ret)
+ return ret;
+
+ attr = t->rx.buf;
+
+ ret = ph->xops->do_xfer(ph, t);
+ if (!ret) {
+ pi->nr_rtc = GET_RTCS_NR(attr->attributes);
+ pi->nr_gpr = GET_GPRS_NR(attr->attributes);
+ }
+
+ ph->xops->xfer_put(ph, t);
+
+ return ret;
+}
+
+static int scmi_imx_bbm_notify(const struct scmi_protocol_handle *ph,
+ u32 src_id, int message_id, bool enable)
+{
+ int ret;
+ struct scmi_xfer *t;
+
+ if (message_id == IMX_BBM_RTC_NOTIFY) {
+ struct scmi_msg_imx_bbm_rtc_notify *rtc_notify;
+
+ ret = ph->xops->xfer_get_init(ph, message_id,
+ sizeof(*rtc_notify), 0, &t);
+ if (ret)
+ return ret;
+
+ rtc_notify = t->tx.buf;
+ rtc_notify->rtc_id = cpu_to_le32(0);
+ rtc_notify->flags =
+ cpu_to_le32(enable ? SCMI_IMX_BBM_NOTIFY_RTC_FLAG : 0);
+ } else if (message_id == IMX_BBM_BUTTON_NOTIFY) {
+ struct scmi_msg_imx_bbm_button_notify *button_notify;
+
+ ret = ph->xops->xfer_get_init(ph, message_id,
+ sizeof(*button_notify), 0, &t);
+ if (ret)
+ return ret;
+
+ button_notify = t->tx.buf;
+ button_notify->flags = cpu_to_le32(enable ? 1 : 0);
+ } else {
+ return -EINVAL;
+ }
+
+ ret = ph->xops->do_xfer(ph, t);
+
+ ph->xops->xfer_put(ph, t);
+ return ret;
+}
+
+static enum scmi_imx_bbm_protocol_cmd evt_2_cmd[] = {
+ IMX_BBM_RTC_NOTIFY,
+ IMX_BBM_BUTTON_NOTIFY
+};
+
+static int scmi_imx_bbm_set_notify_enabled(const struct scmi_protocol_handle *ph,
+ u8 evt_id, u32 src_id, bool enable)
+{
+ int ret, cmd_id;
+
+ if (evt_id >= ARRAY_SIZE(evt_2_cmd))
+ return -EINVAL;
+
+ cmd_id = evt_2_cmd[evt_id];
+ ret = scmi_imx_bbm_notify(ph, src_id, cmd_id, enable);
+ if (ret)
+ pr_debug("FAIL_ENABLED - evt[%X] dom[%d] - ret:%d\n",
+ evt_id, src_id, ret);
+
+ return ret;
+}
+
+static void *scmi_imx_bbm_fill_custom_report(const struct scmi_protocol_handle *ph,
+ u8 evt_id, ktime_t timestamp,
+ const void *payld, size_t payld_sz,
+ void *report, u32 *src_id)
+{
+ const struct scmi_imx_bbm_notify_payld *p = payld;
+ struct scmi_imx_bbm_notif_report *r = report;
+
+ if (sizeof(*p) != payld_sz)
+ return NULL;
+
+ if (evt_id == SCMI_EVENT_IMX_BBM_RTC) {
+ r->is_rtc = true;
+ r->is_button = false;
+ r->timestamp = timestamp;
+ r->rtc_id = le32_get_bits(p->flags, SCMI_IMX_BBM_EVENT_RTC_MASK);
+ r->rtc_evt = le32_get_bits(p->flags, SCMI_IMX_BBM_NOTIFY_RTC_FLAG);
+ dev_dbg(ph->dev, "RTC: %d evt: %x\n", r->rtc_id, r->rtc_evt);
+ *src_id = r->rtc_evt;
+ } else if (evt_id == SCMI_EVENT_IMX_BBM_BUTTON) {
+ r->is_rtc = false;
+ r->is_button = true;
+ r->timestamp = timestamp;
+ dev_dbg(ph->dev, "BBM Button\n");
+ *src_id = 0;
+ } else {
+ WARN_ON_ONCE(1);
+ return NULL;
+ }
+
+ return r;
+}
+
+static const struct scmi_event scmi_imx_bbm_events[] = {
+ {
+ .id = SCMI_EVENT_IMX_BBM_RTC,
+ .max_payld_sz = sizeof(struct scmi_imx_bbm_notify_payld),
+ .max_report_sz = sizeof(struct scmi_imx_bbm_notif_report),
+ },
+ {
+ .id = SCMI_EVENT_IMX_BBM_BUTTON,
+ .max_payld_sz = sizeof(struct scmi_imx_bbm_notify_payld),
+ .max_report_sz = sizeof(struct scmi_imx_bbm_notif_report),
+ },
+};
+
+static const struct scmi_event_ops scmi_imx_bbm_event_ops = {
+ .set_notify_enabled = scmi_imx_bbm_set_notify_enabled,
+ .fill_custom_report = scmi_imx_bbm_fill_custom_report,
+};
+
+static const struct scmi_protocol_events scmi_imx_bbm_protocol_events = {
+ .queue_sz = SCMI_PROTO_QUEUE_SZ,
+ .ops = &scmi_imx_bbm_event_ops,
+ .evts = scmi_imx_bbm_events,
+ .num_events = ARRAY_SIZE(scmi_imx_bbm_events),
+ .num_sources = 1,
+};
+
+static int scmi_imx_bbm_protocol_init(const struct scmi_protocol_handle *ph)
+{
+ u32 version;
+ int ret;
+ struct scmi_imx_bbm_info *binfo;
+
+ ret = ph->xops->version_get(ph, &version);
+ if (ret)
+ return ret;
+
+ dev_info(ph->dev, "NXP SM BBM Version %d.%d\n",
+ PROTOCOL_REV_MAJOR(version), PROTOCOL_REV_MINOR(version));
+
+ binfo = devm_kzalloc(ph->dev, sizeof(*binfo), GFP_KERNEL);
+ if (!binfo)
+ return -ENOMEM;
+
+ ret = scmi_imx_bbm_attributes_get(ph, binfo);
+ if (ret)
+ return ret;
+
+ return ph->set_priv(ph, binfo, version);
+}
+
+static int scmi_imx_bbm_rtc_time_set(const struct scmi_protocol_handle *ph,
+ u32 rtc_id, u64 sec)
+{
+ struct scmi_imx_bbm_info *pi = ph->get_priv(ph);
+ struct scmi_imx_bbm_set_time *cfg;
+ struct scmi_xfer *t;
+ int ret;
+
+ if (rtc_id >= pi->nr_rtc)
+ return -EINVAL;
+
+ ret = ph->xops->xfer_get_init(ph, IMX_BBM_RTC_TIME_SET, sizeof(*cfg), 0, &t);
+ if (ret)
+ return ret;
+
+ cfg = t->tx.buf;
+ cfg->id = cpu_to_le32(rtc_id);
+ cfg->flags = 0;
+ cfg->value_low = lower_32_bits(sec);
+ cfg->value_high = upper_32_bits(sec);
+
+ ret = ph->xops->do_xfer(ph, t);
+
+ ph->xops->xfer_put(ph, t);
+
+ return ret;
+}
+
+static int scmi_imx_bbm_rtc_time_get(const struct scmi_protocol_handle *ph,
+ u32 rtc_id, u64 *value)
+{
+ struct scmi_imx_bbm_info *pi = ph->get_priv(ph);
+ struct scmi_imx_bbm_get_time *cfg;
+ struct scmi_xfer *t;
+ int ret;
+
+ if (rtc_id >= pi->nr_rtc)
+ return -EINVAL;
+
+ ret = ph->xops->xfer_get_init(ph, IMX_BBM_RTC_TIME_GET, sizeof(*cfg),
+ sizeof(u64), &t);
+ if (ret)
+ return ret;
+
+ cfg = t->tx.buf;
+ cfg->id = cpu_to_le32(rtc_id);
+ cfg->flags = 0;
+
+ ret = ph->xops->do_xfer(ph, t);
+ if (!ret)
+ *value = get_unaligned_le64(t->rx.buf);
+
+ ph->xops->xfer_put(ph, t);
+
+ return ret;
+}
+
+static int scmi_imx_bbm_rtc_alarm_set(const struct scmi_protocol_handle *ph,
+ u32 rtc_id, u64 sec)
+{
+ struct scmi_imx_bbm_info *pi = ph->get_priv(ph);
+ struct scmi_imx_bbm_alarm_time *cfg;
+ struct scmi_xfer *t;
+ int ret;
+
+ if (rtc_id >= pi->nr_rtc)
+ return -EINVAL;
+
+ ret = ph->xops->xfer_get_init(ph, IMX_BBM_RTC_ALARM_SET, sizeof(*cfg), 0, &t);
+ if (ret)
+ return ret;
+
+ cfg = t->tx.buf;
+ cfg->id = cpu_to_le32(rtc_id);
+ cfg->flags = SCMI_IMX_BBM_RTC_ALARM_ENABLE_FLAG;
+ cfg->value_low = lower_32_bits(sec);
+ cfg->value_high = upper_32_bits(sec);
+
+ ret = ph->xops->do_xfer(ph, t);
+
+ ph->xops->xfer_put(ph, t);
+
+ return ret;
+}
+
+static int scmi_imx_bbm_button_get(const struct scmi_protocol_handle *ph, u32 *state)
+{
+ struct scmi_xfer *t;
+ int ret;
+
+ ret = ph->xops->xfer_get_init(ph, IMX_BBM_BUTTON_GET, 0, sizeof(u32), &t);
+ if (ret)
+ return ret;
+
+ ret = ph->xops->do_xfer(ph, t);
+ if (!ret)
+ *state = get_unaligned_le32(t->rx.buf);
+
+ ph->xops->xfer_put(ph, t);
+
+ return ret;
+}
+
+static const struct scmi_imx_bbm_proto_ops scmi_imx_bbm_proto_ops = {
+ .rtc_time_get = scmi_imx_bbm_rtc_time_get,
+ .rtc_time_set = scmi_imx_bbm_rtc_time_set,
+ .rtc_alarm_set = scmi_imx_bbm_rtc_alarm_set,
+ .button_get = scmi_imx_bbm_button_get,
+};
+
+static const struct scmi_protocol scmi_imx_bbm = {
+ .id = SCMI_PROTOCOL_IMX_BBM,
+ .owner = THIS_MODULE,
+ .instance_init = &scmi_imx_bbm_protocol_init,
+ .ops = &scmi_imx_bbm_proto_ops,
+ .events = &scmi_imx_bbm_protocol_events,
+ .supported_version = SCMI_PROTOCOL_SUPPORTED_VERSION,
+};
+
+module_scmi_protocol(scmi_imx_bbm);
diff --git a/include/linux/scmi_imx_protocol.h b/include/linux/scmi_imx_protocol.h
new file mode 100644
index 000000000000..90ce011a4429
--- /dev/null
+++ b/include/linux/scmi_imx_protocol.h
@@ -0,0 +1,45 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * SCMI Message Protocol driver NXP extension header
+ *
+ * Copyright 2024 NXP.
+ */
+
+#ifndef _LINUX_SCMI_NXP_PROTOCOL_H
+#define _LINUX_SCMI_NXP_PROTOCOL_H
+
+#include <linux/bitfield.h>
+#include <linux/device.h>
+#include <linux/notifier.h>
+#include <linux/types.h>
+
+enum scmi_nxp_protocol {
+ SCMI_PROTOCOL_IMX_BBM = 0x81,
+};
+
+struct scmi_imx_bbm_proto_ops {
+ int (*rtc_time_set)(const struct scmi_protocol_handle *ph, u32 id,
+ uint64_t sec);
+ int (*rtc_time_get)(const struct scmi_protocol_handle *ph, u32 id,
+ u64 *val);
+ int (*rtc_alarm_set)(const struct scmi_protocol_handle *ph, u32 id,
+ u64 sec);
+ int (*button_get)(const struct scmi_protocol_handle *ph, u32 *state);
+};
+
+enum scmi_nxp_notification_events {
+ SCMI_EVENT_IMX_BBM_RTC = 0x0,
+ SCMI_EVENT_IMX_BBM_BUTTON = 0x1,
+ SCMI_EVENT_IMX_MISC_CONTROL_DISABLED = 0x0,
+ SCMI_EVENT_IMX_MISC_CONTROL_FALLING_EDGE = 0x1,
+ SCMI_EVENT_IMX_MISC_CONTROL_RISING_EDGE = 0x2,
+};
+
+struct scmi_imx_bbm_notif_report {
+ bool is_rtc;
+ bool is_button;
+ ktime_t timestamp;
+ unsigned int rtc_id;
+ unsigned int rtc_evt;
+};
+#endif
--
2.37.1
^ permalink raw reply related
* [PATCH v2 2/6] dt-bindings: firmware: add i.MX SCMI Extension protocol
From: Peng Fan (OSS) @ 2024-04-05 12:39 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
Sudeep Holla, Cristian Marussi
Cc: Peng Fan, devicetree, imx, linux-arm-kernel, linux-kernel
In-Reply-To: <20240405-imx95-bbm-misc-v2-v2-0-9fc9186856c2@nxp.com>
From: Peng Fan <peng.fan@nxp.com>
Add i.MX SCMI Extension protocols bindings for:
- Battery Backed Secure Module(BBSM)
- MISC settings such as General Purpose Registers settings.
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
.../devicetree/bindings/firmware/imx,scmi.yaml | 80 ++++++++++++++++++++++
1 file changed, 80 insertions(+)
diff --git a/Documentation/devicetree/bindings/firmware/imx,scmi.yaml b/Documentation/devicetree/bindings/firmware/imx,scmi.yaml
new file mode 100644
index 000000000000..7ee19a661d83
--- /dev/null
+++ b/Documentation/devicetree/bindings/firmware/imx,scmi.yaml
@@ -0,0 +1,80 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+# Copyright 2024 NXP
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/firmware/imx,scmi.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: i.MX System Control and Management Interface(SCMI) Vendor Protocols Extension
+
+maintainers:
+ - Peng Fan <peng.fan@nxp.com>
+
+allOf:
+ - $ref: arm,scmi.yaml#
+
+properties:
+ protocol@81:
+ $ref: 'arm,scmi.yaml#/$defs/protocol-node'
+ unevaluatedProperties: false
+ description:
+ The BBM Protocol is for managing Battery Backed Secure Module (BBSM) RTC
+ and the ON/OFF Key
+
+ properties:
+ reg:
+ const: 0x81
+
+ required:
+ - reg
+
+ protocol@84:
+ $ref: 'arm,scmi.yaml#/$defs/protocol-node'
+ unevaluatedProperties: false
+ description:
+ The MISC Protocol is for managing SoC Misc settings, such as GPR settings
+
+ properties:
+ reg:
+ const: 0x84
+
+ wakeup-sources:
+ description:
+ Each entry consists of 2 integers, represents the source and electric signal edge
+ items:
+ items:
+ - description: the wakeup source
+ - description: the wakeup electric signal edge
+ $ref: /schemas/types.yaml#/definitions/uint32-matrix
+
+ required:
+ - reg
+
+additionalProperties: false
+
+examples:
+ - |
+ firmware {
+ scmi {
+ compatible = "arm,scmi";
+ mboxes = <&mu2 5 0>, <&mu2 3 0>, <&mu2 3 1>;
+ shmem = <&scmi_buf0>, <&scmi_buf1>;
+
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ protocol@81 {
+ reg = <0x81>;
+ };
+
+ protocol@84 {
+ reg = <0x84>;
+ wakeup-sources = <0x8000 1
+ 0x8001 1
+ 0x8002 1
+ 0x8003 1
+ 0x8004 1>;
+ };
+ };
+ };
+...
--
2.37.1
^ permalink raw reply related
* [PATCH v2 1/6] dt-bindings: firmware: arm,scmi: set additionalProperties to true
From: Peng Fan (OSS) @ 2024-04-05 12:39 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
Sudeep Holla, Cristian Marussi
Cc: Peng Fan, devicetree, imx, linux-arm-kernel, linux-kernel
In-Reply-To: <20240405-imx95-bbm-misc-v2-v2-0-9fc9186856c2@nxp.com>
From: Peng Fan <peng.fan@nxp.com>
When adding vendor extension protocols, there is dt-schema warning:
"
imx,scmi.example.dtb: scmi: 'protocol@81', 'protocol@84' do not match any
of the regexes: 'pinctrl-[0-9]+'
"
Set additionalProperties to true to address the issue.
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
Documentation/devicetree/bindings/firmware/arm,scmi.yaml | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
index 4591523b51a0..cfc613b65585 100644
--- a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
+++ b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
@@ -247,7 +247,7 @@ properties:
reg:
const: 0x18
-additionalProperties: false
+additionalProperties: true
$defs:
protocol-node:
--
2.37.1
^ permalink raw reply related
* [PATCH v2 0/6] firmware: support i.MX95 SCMI BBM/MISC Extenstion
From: Peng Fan (OSS) @ 2024-04-05 12:39 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
Sudeep Holla, Cristian Marussi
Cc: Peng Fan, devicetree, imx, linux-arm-kernel, linux-kernel
i.MX95 System Manager Firmware support vendor extension protocol:
- Battery Backed Module(BBM) Protocol for RTC and BUTTON feature.
- MISC Protocol for misc settings, such as BLK CTRL GPR settings, GPIO
expander settings.
This patchset is to support the two protocols and users that use the
protocols.
Signed-off-by: Peng Fan <peng.fan@nxp.com>
To: Rob Herring <robh@kernel.org>
To: Krzysztof Kozlowski <krzk+dt@kernel.org>
To: Conor Dooley <conor+dt@kernel.org>
To: Shawn Guo <shawnguo@kernel.org>
To: Sascha Hauer <s.hauer@pengutronix.de>
To: Pengutronix Kernel Team <kernel@pengutronix.de>
To: Fabio Estevam <festevam@gmail.com>
To: Peng Fan <peng.fan@nxp.com>
To: Sudeep Holla <sudeep.holla@arm.com>
To: Cristian Marussi <cristian.marussi@arm.com>
Cc: devicetree@vger.kernel.org
Cc: imx@lists.linux.dev
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
Changes in v2:
- Sorry for late update since v1.
- Add a new patch 1
- Address imx,scmi.yaml issues
- Address comments for imx-sm-bbm.c and imx-sm-misc.c
- I not add vendor id since related patches not landed in linux-next.
- Link to v1: https://lore.kernel.org/r/20240202-imx95-bbm-misc-v1-0-3cb743020933@nxp.com
---
Peng Fan (6):
dt-bindings: firmware: arm,scmi: set additionalProperties to true
dt-bindings: firmware: add i.MX SCMI Extension protocol
firmware: arm_scmi: add initial support for i.MX BBM protocol
firmware: arm_scmi: add initial support for i.MX MISC protocol
firmware: imx: support BBM module
firmware: imx: add i.MX95 MISC driver
.../devicetree/bindings/firmware/arm,scmi.yaml | 2 +-
.../devicetree/bindings/firmware/imx,scmi.yaml | 80 +++++
drivers/firmware/arm_scmi/Kconfig | 20 ++
drivers/firmware/arm_scmi/Makefile | 2 +
drivers/firmware/arm_scmi/imx-sm-bbm.c | 378 +++++++++++++++++++++
drivers/firmware/arm_scmi/imx-sm-misc.c | 305 +++++++++++++++++
drivers/firmware/imx/Makefile | 2 +
drivers/firmware/imx/sm-bbm.c | 317 +++++++++++++++++
drivers/firmware/imx/sm-misc.c | 92 +++++
include/linux/firmware/imx/sm.h | 33 ++
include/linux/scmi_imx_protocol.h | 62 ++++
11 files changed, 1292 insertions(+), 1 deletion(-)
---
base-commit: 2b3d5988ae2cb5cd945ddbc653f0a71706231fdd
change-id: 20240405-imx95-bbm-misc-v2-b5e9d24adc42
Best regards,
--
Peng Fan <peng.fan@nxp.com>
^ permalink raw reply
* Re: [PATCH v3 1/9] drm: xlnx: zynqmp_dpsub: Set layer mode during creation
From: Tomi Valkeinen @ 2024-04-05 12:31 UTC (permalink / raw)
To: Anatoliy Klymenko
Cc: dri-devel, linux-arm-kernel, linux-kernel, devicetree,
linux-media, Laurent Pinchart, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Daniel Vetter, Michal Simek,
Andrzej Hajda, Neil Armstrong, Robert Foss, Jonas Karlman,
Jernej Skrabec, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Mauro Carvalho Chehab
In-Reply-To: <20240321-dp-live-fmt-v3-1-d5090d796b7e@amd.com>
On 21/03/2024 22:43, Anatoliy Klymenko wrote:
> Set layer mode of operation (live or dma-based) during layer creation.
>
> Each DPSUB layer mode of operation is defined by corresponding DT node port
> connection, so it is possible to assign it during layer object creation.
> Previously it was set in layer enable functions, although it is too late
> as setting layer format depends on layer mode, and should be done before
> given layer enabled.
>
> Signed-off-by: Anatoliy Klymenko <anatoliy.klymenko@amd.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> drivers/gpu/drm/xlnx/zynqmp_disp.c | 20 ++++++++++++++++----
> drivers/gpu/drm/xlnx/zynqmp_disp.h | 13 +------------
> drivers/gpu/drm/xlnx/zynqmp_dp.c | 2 +-
> drivers/gpu/drm/xlnx/zynqmp_kms.c | 2 +-
> 4 files changed, 19 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/xlnx/zynqmp_disp.c b/drivers/gpu/drm/xlnx/zynqmp_disp.c
> index 8a39b3accce5..e6d26ef60e89 100644
> --- a/drivers/gpu/drm/xlnx/zynqmp_disp.c
> +++ b/drivers/gpu/drm/xlnx/zynqmp_disp.c
> @@ -64,6 +64,16 @@
>
> #define ZYNQMP_DISP_MAX_NUM_SUB_PLANES 3
>
> +/**
> + * enum zynqmp_dpsub_layer_mode - Layer mode
> + * @ZYNQMP_DPSUB_LAYER_NONLIVE: non-live (memory) mode
> + * @ZYNQMP_DPSUB_LAYER_LIVE: live (stream) mode
> + */
> +enum zynqmp_dpsub_layer_mode {
> + ZYNQMP_DPSUB_LAYER_NONLIVE,
> + ZYNQMP_DPSUB_LAYER_LIVE,
> +};
> +
> /**
> * struct zynqmp_disp_format - Display subsystem format information
> * @drm_fmt: DRM format (4CC)
> @@ -902,15 +912,12 @@ u32 *zynqmp_disp_layer_drm_formats(struct zynqmp_disp_layer *layer,
> /**
> * zynqmp_disp_layer_enable - Enable a layer
> * @layer: The layer
> - * @mode: Operating mode of layer
> *
> * Enable the @layer in the audio/video buffer manager and the blender. DMA
> * channels are started separately by zynqmp_disp_layer_update().
> */
> -void zynqmp_disp_layer_enable(struct zynqmp_disp_layer *layer,
> - enum zynqmp_dpsub_layer_mode mode)
> +void zynqmp_disp_layer_enable(struct zynqmp_disp_layer *layer)
> {
> - layer->mode = mode;
> zynqmp_disp_avbuf_enable_video(layer->disp, layer);
> zynqmp_disp_blend_layer_enable(layer->disp, layer);
> }
> @@ -1134,6 +1141,11 @@ static int zynqmp_disp_create_layers(struct zynqmp_disp *disp)
> layer->id = i;
> layer->disp = disp;
> layer->info = &layer_info[i];
> + /* For now assume dpsub works in either live or non-live mode for both layers.
> + * Hybrid mode is not supported yet.
> + */
This comment style is not according to the style guide, and in fact you
fix it in the patch 4. So please fix it here instead.
Tomi
> + layer->mode = disp->dpsub->dma_enabled ? ZYNQMP_DPSUB_LAYER_NONLIVE
> + : ZYNQMP_DPSUB_LAYER_LIVE;
>
> ret = zynqmp_disp_layer_request_dma(disp, layer);
> if (ret)
> diff --git a/drivers/gpu/drm/xlnx/zynqmp_disp.h b/drivers/gpu/drm/xlnx/zynqmp_disp.h
> index 123cffac08be..9b8b202224d9 100644
> --- a/drivers/gpu/drm/xlnx/zynqmp_disp.h
> +++ b/drivers/gpu/drm/xlnx/zynqmp_disp.h
> @@ -42,16 +42,6 @@ enum zynqmp_dpsub_layer_id {
> ZYNQMP_DPSUB_LAYER_GFX,
> };
>
> -/**
> - * enum zynqmp_dpsub_layer_mode - Layer mode
> - * @ZYNQMP_DPSUB_LAYER_NONLIVE: non-live (memory) mode
> - * @ZYNQMP_DPSUB_LAYER_LIVE: live (stream) mode
> - */
> -enum zynqmp_dpsub_layer_mode {
> - ZYNQMP_DPSUB_LAYER_NONLIVE,
> - ZYNQMP_DPSUB_LAYER_LIVE,
> -};
> -
> void zynqmp_disp_enable(struct zynqmp_disp *disp);
> void zynqmp_disp_disable(struct zynqmp_disp *disp);
> int zynqmp_disp_setup_clock(struct zynqmp_disp *disp,
> @@ -62,8 +52,7 @@ void zynqmp_disp_blend_set_global_alpha(struct zynqmp_disp *disp,
>
> u32 *zynqmp_disp_layer_drm_formats(struct zynqmp_disp_layer *layer,
> unsigned int *num_formats);
> -void zynqmp_disp_layer_enable(struct zynqmp_disp_layer *layer,
> - enum zynqmp_dpsub_layer_mode mode);
> +void zynqmp_disp_layer_enable(struct zynqmp_disp_layer *layer);
> void zynqmp_disp_layer_disable(struct zynqmp_disp_layer *layer);
> void zynqmp_disp_layer_set_format(struct zynqmp_disp_layer *layer,
> const struct drm_format_info *info);
> diff --git a/drivers/gpu/drm/xlnx/zynqmp_dp.c b/drivers/gpu/drm/xlnx/zynqmp_dp.c
> index 1846c4971fd8..04b6bcac3b07 100644
> --- a/drivers/gpu/drm/xlnx/zynqmp_dp.c
> +++ b/drivers/gpu/drm/xlnx/zynqmp_dp.c
> @@ -1295,7 +1295,7 @@ static void zynqmp_dp_disp_enable(struct zynqmp_dp *dp,
> /* TODO: Make the format configurable. */
> info = drm_format_info(DRM_FORMAT_YUV422);
> zynqmp_disp_layer_set_format(layer, info);
> - zynqmp_disp_layer_enable(layer, ZYNQMP_DPSUB_LAYER_LIVE);
> + zynqmp_disp_layer_enable(layer);
>
> if (layer_id == ZYNQMP_DPSUB_LAYER_GFX)
> zynqmp_disp_blend_set_global_alpha(dp->dpsub->disp, true, 255);
> diff --git a/drivers/gpu/drm/xlnx/zynqmp_kms.c b/drivers/gpu/drm/xlnx/zynqmp_kms.c
> index db3bb4afbfc4..43bf416b33d5 100644
> --- a/drivers/gpu/drm/xlnx/zynqmp_kms.c
> +++ b/drivers/gpu/drm/xlnx/zynqmp_kms.c
> @@ -122,7 +122,7 @@ static void zynqmp_dpsub_plane_atomic_update(struct drm_plane *plane,
>
> /* Enable or re-enable the plane if the format has changed. */
> if (format_changed)
> - zynqmp_disp_layer_enable(layer, ZYNQMP_DPSUB_LAYER_NONLIVE);
> + zynqmp_disp_layer_enable(layer);
> }
>
> static const struct drm_plane_helper_funcs zynqmp_dpsub_plane_helper_funcs = {
>
^ permalink raw reply
* Re: [PATCH 1/4] arm64: dts: stratix10: socdk: drop unneeded flash address/size-cells
From: Dinh Nguyen @ 2024-04-05 12:30 UTC (permalink / raw)
To: Krzysztof Kozlowski, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, devicetree, linux-kernel
In-Reply-To: <20240401141025.98125-1-krzk@kernel.org>
On 4/1/24 09:10, Krzysztof Kozlowski wrote:
> Flash node uses single "partition" node to describe partitions, so
> remove deprecated address/size-cells properties to also fix dtc W=1
> warnings:
>
> socfpga_stratix10_socdk.dts:182.10-211.4: Warning (avoid_unnecessary_addr_size): /soc@0/spi@ff8d2000/flash@0: unnecessary #address-cells/#size-cells without "ranges" or child "reg" property
>
> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
> ---
> arch/arm64/boot/dts/altera/socfpga_stratix10_socdk.dts | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/altera/socfpga_stratix10_socdk.dts b/arch/arm64/boot/dts/altera/socfpga_stratix10_socdk.dts
> index 26173f0b0051..4eee777ef1a1 100644
> --- a/arch/arm64/boot/dts/altera/socfpga_stratix10_socdk.dts
> +++ b/arch/arm64/boot/dts/altera/socfpga_stratix10_socdk.dts
> @@ -180,8 +180,6 @@ rtc@68 {
> &qspi {
> status = "okay";
> flash@0 {
> - #address-cells = <1>;
> - #size-cells = <1>;
> compatible = "micron,mt25qu02g", "jedec,spi-nor";
> reg = <0>;
> spi-max-frequency = <100000000>;
All patches applied!
Thanks,
Dinh
^ permalink raw reply
* Re: [PATCH v2 02/18] PCI: endpoint: Introduce pci_epc_map_align()
From: Niklas Cassel @ 2024-04-05 12:20 UTC (permalink / raw)
To: Damien Le Moal
Cc: Kishon Vijay Abraham I, Manivannan Sadhasivam, Lorenzo Pieralisi,
Kishon Vijay Abraham I, Shawn Lin, Krzysztof Wilczyński,
Bjorn Helgaas, Heiko Stuebner, linux-pci, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, devicetree, linux-rockchip,
linux-arm-kernel, Rick Wertenbroek, Wilfred Mallawa
In-Reply-To: <3a2aff21-4b1d-4f99-bd49-bf75f41cb924@kernel.org>
On Thu, Apr 04, 2024 at 11:43:47AM +0900, Damien Le Moal wrote:
> On 4/3/24 21:33, Kishon Vijay Abraham I wrote:
> > Hi Damien,
> >
> > On 3/30/2024 9:49 AM, Damien Le Moal wrote:
> >> Some endpoint controllers have requirements on the alignment of the
> >> controller physical memory address that must be used to map a RC PCI
> >> address region. For instance, the rockchip endpoint controller uses
> >> at most the lower 20 bits of a physical memory address region as the
> >> lower bits of an RC PCI address. For mapping a PCI address region of
> >> size bytes starting from pci_addr, the exact number of address bits
> >> used is the number of address bits changing in the address range
> >> [pci_addr..pci_addr + size - 1].
> >>
> >> For this example, this creates the following constraints:
> >> 1) The offset into the controller physical memory allocated for a
> >> mapping depends on the mapping size *and* the starting PCI address
> >> for the mapping.
> >> 2) A mapping size cannot exceed the controller windows size (1MB) minus
> >> the offset needed into the allocated physical memory, which can end
> >> up being a smaller size than the desired mapping size.
> >>
> >> Handling these constraints independently of the controller being used in
> >> a PCI EP function driver is not possible with the current EPC API as
> >> it only provides the ->align field in struct pci_epc_features.
> >> Furthermore, this alignment is static and does not depend on a mapping
> >> pci address and size.
> >>
> >> Solve this by introducing the function pci_epc_map_align() and the
> >> endpoint controller operation ->map_align to allow endpoint function
> >> drivers to obtain the size and the offset into a controller address
> >> region that must be used to map an RC PCI address region. The size
> >> of the physical address region provided by pci_epc_map_align() can then
> >> be used as the size argument for the function pci_epc_mem_alloc_addr().
> >> The offset into the allocated controller memory can be used to
> >> correctly handle data transfers. Of note is that pci_epc_map_align() may
> >> indicate upon return a mapping size that is smaller (but not 0) than the
> >> requested PCI address region size. For such case, an endpoint function
> >> driver must handle data transfers in fragments.
> >>
> >> The controller operation ->map_align is optional: controllers that do
> >> not have any address alignment constraints for mapping a RC PCI address
> >> region do not need to implement this operation. For such controllers,
> >> pci_epc_map_align() always returns the mapping size as equal
> >> to the requested size and an offset equal to 0.
> >>
> >> The structure pci_epc_map is introduced to represent a mapping start PCI
> >> address, size and the size and offset into the controller memory needed
> >> for mapping the PCI address region.
> >>
> >> Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
> >> ---
> >> drivers/pci/endpoint/pci-epc-core.c | 66 +++++++++++++++++++++++++++++
> >> include/linux/pci-epc.h | 33 +++++++++++++++
> >> 2 files changed, 99 insertions(+)
> >>
> >> diff --git a/drivers/pci/endpoint/pci-epc-core.c b/drivers/pci/endpoint/pci-epc-core.c
> >> index 754afd115bbd..37758ca91d7f 100644
> >> --- a/drivers/pci/endpoint/pci-epc-core.c
> >> +++ b/drivers/pci/endpoint/pci-epc-core.c
> >> @@ -433,6 +433,72 @@ void pci_epc_unmap_addr(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
> >> }
> >> EXPORT_SYMBOL_GPL(pci_epc_unmap_addr);
> >>
> >> +/**
> >> + * pci_epc_map_align() - Get the offset into and the size of a controller memory
> >> + * address region needed to map a RC PCI address region
> >> + * @epc: the EPC device on which address is allocated
> >> + * @func_no: the physical endpoint function number in the EPC device
> >> + * @vfunc_no: the virtual endpoint function number in the physical function
> >> + * @pci_addr: PCI address to which the physical address should be mapped
> >> + * @size: the size of the mapping starting from @pci_addr
> >> + * @map: populate here the actual size and offset into the controller memory
> >> + * that must be allocated for the mapping
> >> + *
> >> + * Invoke the controller map_align operation to obtain the size and the offset
> >> + * into a controller address region that must be allocated to map @size
> >> + * bytes of the RC PCI address space starting from @pci_addr.
> >> + *
> >> + * The size of the mapping that can be handled by the controller is indicated
> >> + * using the pci_size field of @map. This size may be smaller than the requested
> >> + * @size. In such case, the function driver must handle the mapping using
> >> + * several fragments. The offset into the controller memory for the effective
> >> + * mapping of the @pci_addr..@pci_addr+@map->pci_size address range is indicated
> >> + * using the map_ofst field of @map.
> >> + */
> >> +int pci_epc_map_align(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
> >> + u64 pci_addr, size_t size, struct pci_epc_map *map)
> >> +{
> >> + const struct pci_epc_features *features;
> >> + size_t mask;
> >> + int ret;
> >> +
> >> + if (!pci_epc_function_is_valid(epc, func_no, vfunc_no))
> >> + return -EINVAL;
> >> +
> >> + if (!size || !map)
> >> + return -EINVAL;
> >> +
> >> + memset(map, 0, sizeof(*map));
> >> + map->pci_addr = pci_addr;
> >> + map->pci_size = size;
> >> +
> >> + if (epc->ops->map_align) {
> >> + mutex_lock(&epc->lock);
> >> + ret = epc->ops->map_align(epc, func_no, vfunc_no, map);
> >> + mutex_unlock(&epc->lock);
> >> + return ret;
> >> + }
> >> +
> >> + /*
> >> + * Assume a fixed alignment constraint as specified by the controller
> >> + * features.
> >> + */
> >> + features = pci_epc_get_features(epc, func_no, vfunc_no);
> >> + if (!features || !features->align) {
> >> + map->map_pci_addr = pci_addr;
> >> + map->map_size = size;
> >> + map->map_ofst = 0;
> >> + }
> >
> > The 'align' of pci_epc_features was initially added only to address the
> > inbound ATU constraints. This is also added as comment in [1]. The PCI
> > address restrictions (only fixed alignment constraint) were handled by
> > the host side driver and depends on the connected endpoint device
> > (atleast it was like that for pci_endpoint_test.c [2]).
> > So pci-epf-test.c used the 'align' in pci_epc_features only as part of
> > pci_epf_alloc_space().
> >
> > Though I have abused 'align' of pci_epc_features in pci-epf-ntb.c using
> > it out of pci_epf_alloc_space(), I think we should keep the 'align' of
> > pci_epc_features only within pci_epf_alloc_space() and controllers with
> > any PCI address restrictions to implement ->map_align(). This could as
> > well be done in a phased manner to let controllers implement
> > ->map_align() and then remove using pci_epc_features in
> > pci_epc_map_align(). Let me know what you think?
First you say that you want to avoid using epc_features->align inside
pci_epc_map_align(), and then you say that we could do it in phases,
and eventually stop using epc_features->align in pci_epc_map_align().
I'm confused... :)
Do you really want pci_epc_map_align() to make use of epc_features->align ?
Don't you mean ep->page_size ?
(Please read the whole email to see my reasoning.)
>
> Yep, good idea. I will remove the use of "align" as a default alignment
> constraint. For controllers that have a fixed alignment constraint (not
> necessarilly epc->features->align), it is trivial to provide a generic helper
> function that implements the ->map_align method.
We can see that commit:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=2a9a801620efac92885fc9cd53594c0b9aba87a4
Introduced epc_features->align and modified pci_epf_alloc_space() to use it.
From reading the commit, it appears that epc_features->align was intended to
represent inbound iATU alignment requirement.
For DWC based controllers, the inbound iATU address must be aligned to:
CX_ATU_MIN_REGION_SIZE.
AFAICT, epc_features->align currently has nothing to do with traffic outbound
from the EP.
For aligning the reads/writes to buffers allocated on the host side,
we currently have .alignment in the host side driver:
https://github.com/torvalds/linux/blob/v6.9-rc2/drivers/misc/pci_endpoint_test.c#L966-L1021
Which should be set to the outbound iATU alignment requirement.
For DWC based controllers, the outbound iATU address must be aligned to:
CX_ATU_MIN_REGION_SIZE.
Additionally, we have ep->page_size, which defines the smallest outbound unit
that can be mapped.
(On DWC based controllers, tis is CX_ATU_MIN_REGION_SIZE.)
ep->page_size is used to specify the outbound alignment for e.g.
dw_pcie_ep_raise_msi_irq() and dw_pcie_ep_raise_msix_irq():
https://github.com/torvalds/linux/blob/v6.9-rc2/drivers/pci/controller/dwc/pcie-designware-ep.c#L488
https://github.com/torvalds/linux/blob/v6.9-rc2/drivers/pci/controller/dwc/pcie-designware-ep.c#L555
which makes sure that we can write to the RC side MSI/MSI-X address
while satisfying the outbound iATU alignment requirement.
See also:
https://lore.kernel.org/linux-pci/20240402-pci2_upstream-v3-2-803414bdb430@nxp.com/
Now I understand that rockchip is the first one that does not have a fixed
alignment.
So for that platform, map_align() will be different from ep->page_size.
(For all DWC based drivers the outbound iATU alignment requirement is
the same as the page size.)
However, it would be nice if:
1) We could have a default implementation of map_align() that by default uses
ep->page_size. Platforms that have non-fixed alignment requirements could
define their own map_align().
2) We fix dw_pcie_ep_raise_msi_irq() and dw_pcie_ep_raise_msix_irq() to use
the new pci_epc_map_align().
3) It is getting too complicated with all these...
epc_features->align, ep->page_size, map_align(), and .alignment in host driver.
I think that we need to document each of these in Documentation/PCI/endpoint/
4) It would be nice if we could set page_size correctly for all the PCI device
and vendor IDs that have defined an .alignment in drivers/misc/pci_endpoint_test.c
in the correct EPC driver. That way, we should be able to completely remove all
.alignment specified in drivers/misc/pci_endpoint_test.c.
5) Unfortunately drivers/misc/pci_endpoint_test.c defines a default alignment
of 4K:
https://github.com/torvalds/linux/blob/v6.9-rc2/drivers/misc/pci_endpoint_test.c#L968
https://github.com/torvalds/linux/blob/v6.9-rc2/drivers/misc/pci_endpoint_test.c#L820
It would be nice if we could get rid of this as well. Or perhaps add an option
to pci_test so that it does not use this 4k alignment, such that we can verify
that pci_epc_map_align() is actually working.
In my opinion 4) is the biggest win with this series, because it means that
we define the alignment in the EPC driver, instead of needing to define it in
each and every host side driver. But right now, this great improvement is not
really visible for someone looking quickly at the current series.
Kind regards,
Niklas
^ permalink raw reply
* Re: [PATCH v3 3/9] drm: xlnx: zynqmp_dpsub: Add connected live layer helper
From: Tomi Valkeinen @ 2024-04-05 12:12 UTC (permalink / raw)
To: Anatoliy Klymenko
Cc: dri-devel, linux-arm-kernel, linux-kernel, devicetree,
linux-media, Laurent Pinchart, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Daniel Vetter, Michal Simek,
Andrzej Hajda, Neil Armstrong, Robert Foss, Jonas Karlman,
Jernej Skrabec, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Mauro Carvalho Chehab
In-Reply-To: <20240321-dp-live-fmt-v3-3-d5090d796b7e@amd.com>
On 21/03/2024 22:43, Anatoliy Klymenko wrote:
> Add a helper function capturing the first connected live display layer
> discovery logic.
>
> Signed-off-by: Anatoliy Klymenko <anatoliy.klymenko@amd.com>
> ---
> drivers/gpu/drm/xlnx/zynqmp_dp.c | 37 +++++++++++++++++++++++--------------
> 1 file changed, 23 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/xlnx/zynqmp_dp.c b/drivers/gpu/drm/xlnx/zynqmp_dp.c
> index 04b6bcac3b07..4faafdd76798 100644
> --- a/drivers/gpu/drm/xlnx/zynqmp_dp.c
> +++ b/drivers/gpu/drm/xlnx/zynqmp_dp.c
> @@ -1276,28 +1276,40 @@ static void zynqmp_dp_encoder_mode_set_stream(struct zynqmp_dp *dp,
> * DISP Configuration
> */
>
> +/**
> + * zynqmp_dp_disp_connected_live_layer - Return the first connected live layer
> + * @dp: DisplayPort IP core structure
> + *
> + * Return: The first connected live display layer or NULL if none of the live
> + * layer is connected.
"layers"
Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Tomi
> + */
> +static struct zynqmp_disp_layer *
> +zynqmp_dp_disp_connected_live_layer(struct zynqmp_dp *dp)
> +{
> + if (dp->dpsub->connected_ports & BIT(ZYNQMP_DPSUB_PORT_LIVE_VIDEO))
> + return dp->dpsub->layers[ZYNQMP_DPSUB_LAYER_VID];
> + else if (dp->dpsub->connected_ports & BIT(ZYNQMP_DPSUB_PORT_LIVE_GFX))
> + return dp->dpsub->layers[ZYNQMP_DPSUB_LAYER_GFX];
> + else
> + return NULL;
> +}
> +
> static void zynqmp_dp_disp_enable(struct zynqmp_dp *dp,
> struct drm_bridge_state *old_bridge_state)
> {
> - enum zynqmp_dpsub_layer_id layer_id;
> struct zynqmp_disp_layer *layer;
> const struct drm_format_info *info;
>
> - if (dp->dpsub->connected_ports & BIT(ZYNQMP_DPSUB_PORT_LIVE_VIDEO))
> - layer_id = ZYNQMP_DPSUB_LAYER_VID;
> - else if (dp->dpsub->connected_ports & BIT(ZYNQMP_DPSUB_PORT_LIVE_GFX))
> - layer_id = ZYNQMP_DPSUB_LAYER_GFX;
> - else
> + layer = zynqmp_dp_disp_connected_live_layer(dp);
> + if (!layer)
> return;
>
> - layer = dp->dpsub->layers[layer_id];
> -
> /* TODO: Make the format configurable. */
> info = drm_format_info(DRM_FORMAT_YUV422);
> zynqmp_disp_layer_set_format(layer, info);
> zynqmp_disp_layer_enable(layer);
>
> - if (layer_id == ZYNQMP_DPSUB_LAYER_GFX)
> + if (layer == dp->dpsub->layers[ZYNQMP_DPSUB_LAYER_GFX])
> zynqmp_disp_blend_set_global_alpha(dp->dpsub->disp, true, 255);
> else
> zynqmp_disp_blend_set_global_alpha(dp->dpsub->disp, false, 0);
> @@ -1310,11 +1322,8 @@ static void zynqmp_dp_disp_disable(struct zynqmp_dp *dp,
> {
> struct zynqmp_disp_layer *layer;
>
> - if (dp->dpsub->connected_ports & BIT(ZYNQMP_DPSUB_PORT_LIVE_VIDEO))
> - layer = dp->dpsub->layers[ZYNQMP_DPSUB_LAYER_VID];
> - else if (dp->dpsub->connected_ports & BIT(ZYNQMP_DPSUB_PORT_LIVE_GFX))
> - layer = dp->dpsub->layers[ZYNQMP_DPSUB_LAYER_GFX];
> - else
> + layer = zynqmp_dp_disp_connected_live_layer(dp);
> + if (!layer)
> return;
>
> zynqmp_disp_disable(dp->dpsub->disp);
>
^ permalink raw reply
* Re: [PATCH RFT 10/10] arm64: dts: microchip: sparx5_pcb135: drop duplicated NOR flash
From: Steen Hegelund @ 2024-04-05 12:11 UTC (permalink / raw)
To: Krzysztof Kozlowski, Conor Dooley, Nicolas Ferre, Claudiu Beznea,
Rob Herring, Krzysztof Kozlowski, Lars Povlsen, Daniel Machon,
UNGLinuxDriver, David S. Miller, Bjarni Jonasson,
linux-arm-kernel, devicetree, linux-kernel
In-Reply-To: <20240401153740.123978-10-krzk@kernel.org>
Hi Krzysztof,
On Mon, 2024-04-01 at 17:37 +0200, Krzysztof Kozlowski wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> Since beginning the DTS extended the SPI0 in two places adding two SPI
> muxes, each with same SPI NOR flash. Both used exactly the same
> chip-selects, so this was clearly buggy code. Without checking in
> datasheet, assume device has only one SPI NOR flash, so code was
> duplicated.
>
> Fixes dtc W=1 warnings:
>
> sparx5_pcb135_board.dtsi:92.10-96.4: Warning (unique_unit_address_if_enabled):
> /axi@600000000/spi@600104000/flash@0: duplicate unit-address (also used in node
> /axi@600000000/spi@600104000/spi@0)
>
> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
>
> ---
>
> Not tested on hardware
> ---
> .../boot/dts/microchip/sparx5_pcb135_board.dtsi | 16 ----------------
> 1 file changed, 16 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/microchip/sparx5_pcb135_board.dtsi
> b/arch/arm64/boot/dts/microchip/sparx5_pcb135_board.dtsi
> index 20016efb3656..d64e642e3873 100644
> --- a/arch/arm64/boot/dts/microchip/sparx5_pcb135_board.dtsi
> +++ b/arch/arm64/boot/dts/microchip/sparx5_pcb135_board.dtsi
> @@ -96,22 +96,6 @@ flash@0 {
> };
> };
>
> -&spi0 {
> - status = "okay";
> - spi@0 {
> - compatible = "spi-mux";
> - mux-controls = <&mux>;
> - #address-cells = <1>;
> - #size-cells = <0>;
> - reg = <0>; /* CS0 */
> - flash@9 {
> - compatible = "jedec,spi-nor";
> - spi-max-frequency = <8000000>;
> - reg = <0x9>; /* SPI */
> - };
> - };
> -};
> -
I also tested this, and no surprise: same comment as for the pcb134 patch...
> &sgpio1 {
> status = "okay";
> microchip,sgpio-port-ranges = <24 31>;
> --
> 2.34.1
>
Best Regards
Steen
^ permalink raw reply
* Re: [PATCH RFT 09/10] arm64: dts: microchip: sparx5_pcb134: drop duplicated NOR flash
From: Steen Hegelund @ 2024-04-05 12:10 UTC (permalink / raw)
To: Krzysztof Kozlowski, Conor Dooley, Nicolas Ferre, Claudiu Beznea,
Rob Herring, Krzysztof Kozlowski, Lars Povlsen, Daniel Machon,
UNGLinuxDriver, David S. Miller, Bjarni Jonasson,
linux-arm-kernel, devicetree, linux-kernel
In-Reply-To: <20240401153740.123978-9-krzk@kernel.org>
Hi Krzysztof,
On Mon, 2024-04-01 at 17:37 +0200, Krzysztof Kozlowski wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> Since beginning the DTS extended the SPI0 in two places adding two SPI
> muxes, each with same SPI NOR flash. Both used exactly the same
> chip-selects, so this was clearly buggy code. Without checking in
> datasheet, assume device has only one SPI NOR flash, so code was
> duplicated.
>
> Fixes dtc W=1 warnings:
>
> sparx5_pcb134_board.dtsi:277.10-281.4: Warning (unique_unit_address_if_enabled):
> /axi@600000000/spi@600104000/flash@0: duplicate unit-address (also used in node
> /axi@600000000/spi@600104000/spi@0)
>
> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
>
> ---
>
> Not tested on hardware
> ---
> .../boot/dts/microchip/sparx5_pcb134_board.dtsi | 16 ----------------
> 1 file changed, 16 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/microchip/sparx5_pcb134_board.dtsi
> b/arch/arm64/boot/dts/microchip/sparx5_pcb134_board.dtsi
> index f165a409bc1d..dc7b59dfcb40 100644
> --- a/arch/arm64/boot/dts/microchip/sparx5_pcb134_board.dtsi
> +++ b/arch/arm64/boot/dts/microchip/sparx5_pcb134_board.dtsi
> @@ -281,22 +281,6 @@ flash@0 {
> };
> };
>
> -&spi0 {
> - status = "okay";
> - spi@0 {
> - compatible = "spi-mux";
> - mux-controls = <&mux>;
> - #address-cells = <1>;
> - #size-cells = <0>;
> - reg = <0>; /* CS0 */
> - flash@9 {
> - compatible = "jedec,spi-nor";
> - spi-max-frequency = <8000000>;
> - reg = <0x9>; /* SPI */
> - };
> - };
> -};
> -
When testing this on actual HW the SPI NOR is no longer accessible.
The reason is that it sits behind a SPI-MUX and that needs to be present in the Device Tree.
So if you do the "reverse" clean-up it works fine: Remove the simple spi0 node and keep the one that
has the spi-mux reference.
> &sgpio0 {
> status = "okay";
> microchip,sgpio-port-ranges = <8 15>;
> --
> 2.34.1
>
Thanks for the cleanup of the DT files!
Best Regards
Steen
^ permalink raw reply
* Re: [PATCH v3 2/9] drm: xlnx: zynqmp_dpsub: Update live format defines
From: Tomi Valkeinen @ 2024-04-05 12:10 UTC (permalink / raw)
To: Anatoliy Klymenko
Cc: dri-devel, linux-arm-kernel, linux-kernel, devicetree,
linux-media, Laurent Pinchart, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Daniel Vetter, Michal Simek,
Andrzej Hajda, Neil Armstrong, Robert Foss, Jonas Karlman,
Jernej Skrabec, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Mauro Carvalho Chehab
In-Reply-To: <20240321-dp-live-fmt-v3-2-d5090d796b7e@amd.com>
On 21/03/2024 22:43, Anatoliy Klymenko wrote:
> Update live format defines to match DPSUB AV_BUF_LIVE_VID_CONFIG register
> layout.
I think this description needs a bit more. Mention that the defines are
not currently used, so we can change them like this without any other
change.
Tomi
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Anatoliy Klymenko <anatoliy.klymenko@amd.com>
> ---
> drivers/gpu/drm/xlnx/zynqmp_disp_regs.h | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/xlnx/zynqmp_disp_regs.h b/drivers/gpu/drm/xlnx/zynqmp_disp_regs.h
> index f92a006d5070..fa3935384834 100644
> --- a/drivers/gpu/drm/xlnx/zynqmp_disp_regs.h
> +++ b/drivers/gpu/drm/xlnx/zynqmp_disp_regs.h
> @@ -165,10 +165,10 @@
> #define ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_BPC_10 0x2
> #define ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_BPC_12 0x3
> #define ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_BPC_MASK GENMASK(2, 0)
> -#define ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_FMT_RGB 0x0
> -#define ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_FMT_YUV444 0x1
> -#define ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_FMT_YUV422 0x2
> -#define ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_FMT_YONLY 0x3
> +#define ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_FMT_RGB (0x0 << 4)
> +#define ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_FMT_YUV444 (0x1 << 4)
> +#define ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_FMT_YUV422 (0x2 << 4)
> +#define ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_FMT_YONLY (0x3 << 4)
> #define ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_FMT_MASK GENMASK(5, 4)
> #define ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_CB_FIRST BIT(8)
> #define ZYNQMP_DISP_AV_BUF_PALETTE_MEMORY 0x400
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox