Linux Documentation
 help / color / mirror / Atom feed
* Re: [PATCH v5 01/18] kunit: test: add KUnit test runner core
From: Stephen Boyd @ 2019-06-20  0:15 UTC (permalink / raw)
  To: Brendan Higgins, frowand.list, gregkh, jpoimboe, keescook,
	kieran.bingham, mcgrof, peterz, robh, shuah, tytso,
	yamada.masahiro
  Cc: devicetree, dri-devel, kunit-dev, linux-doc, linux-fsdevel,
	linux-kbuild, linux-kernel, linux-kselftest, linux-nvdimm,
	linux-um, Alexander.Levin, Tim.Bird, amir73il, dan.carpenter,
	daniel, jdike, joel, julia.lawall, khilman, knut.omang, logang,
	mpe, pmladek, rdunlap, richard, rientjes, rostedt, wfg,
	Brendan Higgins
In-Reply-To: <20190617082613.109131-2-brendanhiggins@google.com>

Quoting Brendan Higgins (2019-06-17 01:25:56)
> diff --git a/kunit/test.c b/kunit/test.c
> new file mode 100644
> index 0000000000000..d05d254f1521f
> --- /dev/null
> +++ b/kunit/test.c
> @@ -0,0 +1,210 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Base unit test (KUnit) API.
> + *
> + * Copyright (C) 2019, Google LLC.
> + * Author: Brendan Higgins <brendanhiggins@google.com>
> + */
> +
> +#include <linux/sched/debug.h>
> +#include <kunit/test.h>
> +
> +static bool kunit_get_success(struct kunit *test)
> +{
> +       unsigned long flags;
> +       bool success;
> +
> +       spin_lock_irqsave(&test->lock, flags);
> +       success = test->success;
> +       spin_unlock_irqrestore(&test->lock, flags);

I still don't understand the locking scheme in this code. Is the
intention to make getter and setter APIs that are "safe" by adding in a
spinlock that is held around getting and setting various members in the
kunit structure?

In what situation is there more than one thread reading or writing the
kunit struct? Isn't it only a single process that is going to be
operating on this structure? And why do we need to disable irqs? Are we
expecting to be modifying the unit tests from irq contexts?

> +
> +       return success;
> +}
> +
> +static void kunit_set_success(struct kunit *test, bool success)
> +{
> +       unsigned long flags;
> +
> +       spin_lock_irqsave(&test->lock, flags);
> +       test->success = success;
> +       spin_unlock_irqrestore(&test->lock, flags);
> +}
> +
> +static int kunit_vprintk_emit(int level, const char *fmt, va_list args)
> +{
> +       return vprintk_emit(0, level, NULL, 0, fmt, args);
> +}
> +
> +static int kunit_printk_emit(int level, const char *fmt, ...)
> +{
> +       va_list args;
> +       int ret;
> +
> +       va_start(args, fmt);
> +       ret = kunit_vprintk_emit(level, fmt, args);
> +       va_end(args);
> +
> +       return ret;
> +}
> +
> +static void kunit_vprintk(const struct kunit *test,
> +                         const char *level,
> +                         struct va_format *vaf)
> +{
> +       kunit_printk_emit(level[1] - '0', "\t# %s: %pV", test->name, vaf);
> +}
> +
> +static bool kunit_has_printed_tap_version;

Can you please move this into function local scope in the function
below?

> +
> +static void kunit_print_tap_version(void)
> +{
> +       if (!kunit_has_printed_tap_version) {
> +               kunit_printk_emit(LOGLEVEL_INFO, "TAP version 14\n");
> +               kunit_has_printed_tap_version = true;
> +       }
> +}
> +
[...]
> +
> +static bool kunit_module_has_succeeded(struct kunit_module *module)
> +{
> +       const struct kunit_case *test_case;
> +       bool success = true;
> +
> +       for (test_case = module->test_cases; test_case->run_case; test_case++)
> +               if (!test_case->success) {
> +                       success = false;
> +                       break;

Why not 'return false'?

> +               }
> +
> +       return success;

And 'return true'?

> +}
> +
> +static size_t kunit_module_counter = 1;
> +
> +static void kunit_print_subtest_end(struct kunit_module *module)
> +{
> +       kunit_print_ok_not_ok(false,
> +                             kunit_module_has_succeeded(module),
> +                             kunit_module_counter++,
> +                             module->name);
> +}
> +
> +static void kunit_print_test_case_ok_not_ok(struct kunit_case *test_case,
> +                                           size_t test_number)
> +{
> +       kunit_print_ok_not_ok(true,
> +                             test_case->success,
> +                             test_number,
> +                             test_case->name);
> +}
> +
> +void kunit_init_test(struct kunit *test, const char *name)
> +{
> +       spin_lock_init(&test->lock);
> +       test->name = name;
> +       test->success = true;
> +}
> +
> +/*
> + * Performs all logic to run a test case.
> + */
> +static void kunit_run_case(struct kunit_module *module,
> +                          struct kunit_case *test_case)
> +{
> +       struct kunit test;
> +       int ret = 0;
> +
> +       kunit_init_test(&test, test_case->name);
> +
> +       if (module->init) {
> +               ret = module->init(&test);
> +               if (ret) {
> +                       kunit_err(&test, "failed to initialize: %d\n", ret);
> +                       kunit_set_success(&test, false);
> +                       return;
> +               }
> +       }
> +
> +       if (!ret)
> +               test_case->run_case(&test);

Do we need this if condition? ret can only be set to non-zero above but
then we'll exit the function early so it seems unnecessary. Given that,
ret should probably be moved into the module->init path.

> +
> +       if (module->exit)
> +               module->exit(&test);
> +
> +       test_case->success = kunit_get_success(&test);
> +}
> +

^ permalink raw reply

* Re: [PATCH v1 12/22] docs: driver-api: add .rst files from the main dir
From: Peter Zijlstra @ 2019-06-19 21:27 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Linux Doc Mailing List, Jonathan Corbet, linux-kernel
In-Reply-To: <20190619101922.04340605@coco.lan>

On Wed, Jun 19, 2019 at 10:19:22AM -0300, Mauro Carvalho Chehab wrote:
> (c/c list cleaned)
> 
> Em Wed, 19 Jun 2019 13:43:56 +0200
> Peter Zijlstra <peterz@infradead.org> escreveu:
> 
> > On Tue, Jun 18, 2019 at 05:53:17PM -0300, Mauro Carvalho Chehab wrote:
> > 
> > >  .../{ => driver-api}/atomic_bitops.rst        |  2 -  
> > 
> > That's a .txt file, big fat NAK for making it an rst.
> 
> Rst is a text file. This one is parsed properly by Sphinx without
> any changes.

In my tree it is a .txt file, I've not seen patches changing it. And I
disagree, rst is just as much 'a text file' as .c is.

> > >  .../{ => driver-api}/futex-requeue-pi.rst     |  2 -  
> > 
> > >  .../{ => driver-api}/gcc-plugins.rst          |  2 -  
> > 
> > >  Documentation/{ => driver-api}/kprobes.rst    |  2 -
> > >  .../{ => driver-api}/percpu-rw-semaphore.rst  |  2 -  
> > 
> > More NAK for rst conversion
> 
> Again, those don't need any conversion. Those files already parse 
> as-is by Sphinx, with no need for any change.

And yet, they're a .txt file in my tree. And I've not seen a rename,
just this move.

> The only change here is that, on patch 1/22, the files that
> aren't listed on an index file got a :orphan: added in order
> to make this explicit. This patch removes it.

I've no idea what :orphan: is. Text file don't have markup.

> > >  Documentation/{ => driver-api}/pi-futex.rst   |  2 -
> > >  .../{ => driver-api}/preempt-locking.rst      |  2 -  
> > 
> > >  Documentation/{ => driver-api}/rbtree.rst     |  2 -  
> > 
> > >  .../{ => driver-api}/robust-futex-ABI.rst     |  2 -
> > >  .../{ => driver-api}/robust-futexes.rst       |  2 -  
> > 
> > >  .../{ => driver-api}/speculation.rst          |  8 +--
> > >  .../{ => driver-api}/static-keys.rst          |  2 -  
> > 
> > >  .../{ => driver-api}/this_cpu_ops.rst         |  2 -  
> > 
> > >  Documentation/locking/rt-mutex.rst            |  2 +-  
> > 
> > NAK. None of the above have anything to do with driver-api.
> 
> Ok. Where do you think they should sit instead? core-api?

Pretty much all of then are core-api I tihnk, with exception of the one
that are ABI, which have nothing to do with API. And i've no idea where
GCC plugins go, but it's definitely nothing to do with drivers.

Many of the futex ones are about the sys_futex user API, which
apparently we have Documentation/userspace-api/ for.

Why are you doing this if you've no clue what they're on about?

Just randomly moving files about isn't helpful.


^ permalink raw reply

* Re: [PATCH v1] dma-mapping: Fix filename references
From: Bjorn Helgaas @ 2019-06-19 17:57 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86,
	H. Peter Anvin, Tony Luck, Fenghua Yu, Konrad Rzeszutek Wilk,
	Linux Doc Mailing List, Christoph Hellwig
In-Reply-To: <20190619141307.GP9224@smile.fi.intel.com>

On Wed, Jun 19, 2019 at 9:13 AM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Wed, Mar 20, 2019 at 04:31:17PM -0500, Bjorn Helgaas wrote:
> > On Wed, Mar 20, 2019 at 11:13 AM Andy Shevchenko
> > <andriy.shevchenko@linux.intel.com> wrote:
> > >
> > > After the commit cf65a0f6f6ff
> > >
> > >   ("dma-mapping: move all DMA mapping code to kernel/dma")
> > >
> > > some of the files are referring to outdated information, i.e. old file names
> > > of DMA mapping sources.
> > >
> > > Fix it here.
>
> Bjorn, thanks for review, my answers below.
>
> > >   * This function checks if the reserved crashkernel is allowed on the specific
> > >   * IA64 machine flavour. Machines without an IO TLB use swiotlb and require
> > >   * some memory below 4 GB (i.e. in 32 bit area), see the implementation of
> > > - * lib/swiotlb.c. The hpzx1 architecture has an IO TLB but cannot use that
> > > + * kernel/dma/swiotlb.c. The hpzx1 architecture has an IO TLB but cannot use that
> > >   * in kdump case. See the comment in sba_init() in sba_iommu.c.
> >
> > Is the point here that just that if you lack an IOTLB and want devices
> > to be able to reach system memory above 4GB, you need a bounce buffer
> > below 4GB?  If so, maybe we could just say *that* instead of a
> > nebulous reference to "the implementation of */swiotlb.c", which
> > doesn't tell you what part of the implementation is relevant.
>
> This patch is about broken links.

Oh, of course!  Sorry, I don't know what I was thinking.  I was
wasting my time (and yours) with comments about things you're not
changing, sorry about that.

Bjorn

^ permalink raw reply

* Re: [PATCH 12/14] doc-rst: add ABI documentation to the admin-guide book
From: Mauro Carvalho Chehab @ 2019-06-19 16:37 UTC (permalink / raw)
  To: Jani Nikula
  Cc: Greg Kroah-Hartman, Linux Doc Mailing List, Mauro Carvalho Chehab,
	Mauro Carvalho Chehab, linux-kernel, Jonathan Corbet
In-Reply-To: <87h88nth3v.fsf@intel.com>

Em Tue, 18 Jun 2019 11:47:32 +0300
Jani Nikula <jani.nikula@linux.intel.com> escreveu:

> On Mon, 17 Jun 2019, Mauro Carvalho Chehab <mchehab+samsung@kernel.org> wrote:
> > Yeah, I guess it should be possible to do that. How a python script
> > can identify if it was called by Sphinx, or if it was called directly?  
> 
> if __name__ == '__main__':
> 	# run on the command-line, not imported

Ok, when I have some spare time, I may try to convert one script
to python and see how it behaves. 

Thanks,
Mauro

^ permalink raw reply

* Re: [PATCH 04/14] ABI: better identificate tables
From: Mauro Carvalho Chehab @ 2019-06-19 16:14 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Johan Hovold, Linux Doc Mailing List, Mauro Carvalho Chehab,
	Mauro Carvalho Chehab, linux-kernel, Jonathan Corbet,
	Stefan Achatz
In-Reply-To: <20190619150207.GA19346@kroah.com>

Em Wed, 19 Jun 2019 17:02:07 +0200
Greg Kroah-Hartman <gregkh@linuxfoundation.org> escreveu:

> On Wed, Jun 19, 2019 at 10:56:33AM -0300, Mauro Carvalho Chehab wrote:
> > Hi Johan,
> > 
> > Em Wed, 19 Jun 2019 14:51:35 +0200
> > Johan Hovold <johan@kernel.org> escreveu:
> >   
> > > On Thu, Jun 13, 2019 at 11:04:10PM -0300, Mauro Carvalho Chehab wrote:  
> > > > From: Mauro Carvalho Chehab <mchehab@s-opensource.com>
> > > > 
> > > > When parsing via script, it is important to know if the script
> > > > should consider a description as a literal block that should
> > > > be displayed as-is, or if the description can be considered
> > > > as a normal text.
> > > > 
> > > > Change descriptions to ensure that the preceding line of a table
> > > > ends with a colon. That makes easy to identify the need of a
> > > > literal block.    
> > > 
> > > In the cover letter you say that the first four patches of this series,
> > > including this one, "fix some ABI descriptions that are violating the
> > > syntax described at Documentation/ABI/README". This seems a bit harsh,
> > > given that it's you that is now *introducing* a new syntax requirement
> > > to assist your script.  
> > 
> > Yeah, what's there at the cover letter doesn't apply to this specific
> > patch. The thing is that I wrote this series a lot of time ago (2016/17).
> > 
> > I revived those per a request at KS ML, as we still need to expose the
> > ABI content on some book that will be used by userspace people.
> > 
> > So, I just rebased it on the top of curent Kernel, add a cover letter
> > with the things I remembered and re-sent.
> > 
> > In the specific case of this patch, the ":" there actually makes sense
> > for someone that it is reading it as a text file, and it is an easy
> > hack to make it parse better.
> >   
> > > Specifically, this new requirement isn't documented anywhere AFAICT, so
> > > how will anyone adding new ABI descriptions learn about it?  
> > 
> > Yeah, either that or provide an alternative to "Description" tag, to be
> > used with more complex ABI descriptions.
> > 
> > One of the things that occurred to me, back on 2017, is that we should
> > have a way to to specify that an specific ABI description would have
> > a rich format. Something like:
> > 
> > What:		/sys/bus/usb/devices/<busnum>-<devnum>:<config num>.<interface num>/<hid-bus>:<vendor-id>:<product-id>.<num>/pyra/roccatpyra<minor>/actual_cpi
> > Date:		August 2010
> > Contact:	Stefan Achatz <erazor_de@users.sourceforge.net>
> > RST-Description:
> > 		It is possible to switch the cpi setting of the mouse with the
> > 		press of a button.
> > 		When read, this file returns the raw number of the actual cpi
> > 		setting reported by the mouse. This number has to be further
> > 		processed to receive the real dpi value:
> > 
> > 		===== =====
> > 		VALUE DPI
> > 		===== =====
> > 		1     400
> > 		2     800
> > 		4     1600
> > 		===== =====
> > 
> > With that, the script will know that the description contents will be using
> > the ReST markup, and parse it accordingly. Right now, what it does, instead,
> > is to place the description on a code-block, e. g. it will produce this
> > output for the description:
> > 
> > ::
> > 
> > 		It is possible to switch the cpi setting of the mouse with the
> > 		press of a button.
> > 		When read, this file returns the raw number of the actual cpi
> > 		setting reported by the mouse. This number has to be further
> > 		processed to receive the real dpi value:
> > 
> > 		VALUE DPI
> > 		1     400
> > 		2     800
> > 		4     1600
> > 
> > 
> > Greg, 
> > 
> > what do you think?  
> 
> I don't know when "Description" and "RST-Description" would be used.
> Why not just parse "Description" like rst text and if things are "messy"
> we fix them up as found, like you did with the ":" here?  It doesn't
> have to be complex, we can always fix them up after-the-fact if new
> stuff gets added that doesn't quite parse properly.
> 
> Just like we do for most kernel-doc formatting :)

Works for me. Yet, I guess I tried that, back on 2017. 

If I'm not mistaken, the initial patchset to solve the broken things 
won't be small, and will be require a lot of attention in order to
identify what's broken and where.

Btw, one thing is to pass at ReST validation. Another thing is to
produce something that people can read. 

Right now, the pertinent logic at the script I wrote (scripts/get_abi.pl)
is here:

                if (!($desc =~ /^\s*$/)) {
                        if ($desc =~ m/\:\n/ || $desc =~ m/\n[\t ]+/  || $desc =~ m/[\x00-\x08\x0b-\x1f\x7b-\xff]/) {
                                # put everything inside a code block
                                $desc =~ s/\n/\n /g;

                                print "::\n\n";
                                print " $desc\n\n";
                        } else {
                                # Escape any special chars from description
                                $desc =~s/([\x00-\x08\x0b-\x1f\x21-\x2a\x2d\x2f\x3c-\x40\x5c\x5e-\x60\x7b-\xff])/\\$1/g;

                                print "$desc\n\n";
                        }
                }

If it discovers something weird enough, it just places everything
into a comment block. Otherwise, it assumes that it is a plain
text and that any special characters should be escaped.

If the above block is replaced by a simple:

		print "$desc\n\n";

The description content will be handled as a ReST file.

I don't have any time right now to do this change and to handle the
warnings that will start to popup.

Btw, a single replace there is enough to show the amount of problems that
it will rise, as it will basically break Sphinx build with:

	reading sources... [  1%] admin-guide/abi-testing
	reST markup error:
	get_abi.pl rest --dir $srctree/Documentation/ABI/testing:45261: (SEVERE/4) Missing matching underline for section title overline.
	
	==========================
	PCIe Device AER statistics
	These attributes show up under all the devices that are AER capable. These

Thanks,
Mauro

diff --git a/scripts/get_abi.pl b/scripts/get_abi.pl
index 7bc619b6890c..e75f441afdcd 100755
--- a/scripts/get_abi.pl
+++ b/scripts/get_abi.pl
@@ -288,18 +288,18 @@ sub output_rest {
 		$desc =~ s/\n[\-\*\=\^\~]+\n/\n/g;
 
 		if (!($desc =~ /^\s*$/)) {
-			if ($desc =~ m/\:\n/ || $desc =~ m/\n[\t ]+/  || $desc =~ m/[\x00-\x08\x0b-\x1f\x7b-\xff]/) {
-				# put everything inside a code block
-				$desc =~ s/\n/\n /g;
+#			if ($desc =~ m/\:\n/ || $desc =~ m/\n[\t ]+/  || $desc =~ m/[\x00-\x08\x0b-\x1f\x7b-\xff]/) {
+#				# put everything inside a code block
+#				$desc =~ s/\n/\n /g;
 
-				print "::\n\n";
-				print " $desc\n\n";
-			} else {
-				# Escape any special chars from description
-				$desc =~s/([\x00-\x08\x0b-\x1f\x21-\x2a\x2d\x2f\x3c-\x40\x5c\x5e-\x60\x7b-\xff])/\\$1/g;
+#				print "::\n\n";
+#				print " $desc\n\n";
+#			} else {
+#				# Escape any special chars from description
+#				$desc =~s/([\x00-\x08\x0b-\x1f\x21-\x2a\x2d\x2f\x3c-\x40\x5c\x5e-\x60\x7b-\xff])/\\$1/g;
 
 				print "$desc\n\n";
-			}
+#			}
 		} else {
 			print "DESCRIPTION MISSING for $what\n\n" if (!$data{$what}->{is_file});
 		}


^ permalink raw reply related

* Re: [PATCH v1 12/22] docs: driver-api: add .rst files from the main dir
From: Mauro Carvalho Chehab @ 2019-06-19 15:54 UTC (permalink / raw)
  To: Jonathan Corbet
  Cc: David Howells, Linux Doc Mailing List, Linux MM,
	Linux Kernel Mailing List, Asmaa Mnebhi, Vladimir Oltean
In-Reply-To: <20190619085458.08872dbb@lwn.net>

Em Wed, 19 Jun 2019 08:54:58 -0600
Jonathan Corbet <corbet@lwn.net> escreveu:

> [Trimming the CC list from hell made sense, but it might have been better
> to leave me on it...]
> 
> On Wed, 19 Jun 2019 11:15:28 -0300
> Mauro Carvalho Chehab <mchehab+samsung@kernel.org> wrote:
> 
> > Em Wed, 19 Jun 2019 14:39:10 +0100
> > David Howells <dhowells@redhat.com> escreveu:
> >   
> > > Mauro Carvalho Chehab <mchehab+samsung@kernel.org> wrote:
> > >     
> > > > > > -Documentation/nommu-mmap.rst
> > > > > > +Documentation/driver-api/nommu-mmap.rst        
> > > 
> > > Why is this moving to Documentation/driver-api?      
> > 
> > Good point. I tried to do my best with those document renames, but
> > I'm pretty sure some of them ended by going to the wrong place - or
> > at least there are arguments in favor of moving it to different
> > places :-)  
> 
> I think that a lot of this might also be an argument for slowing down just
> a little bit.  I really don't think that blasting through and reformatting
> all of our text documents is the most urgent problem right now and, in
> cases like this, it might create others.
> 
> Organization of the documentation tree is important; it has never really
> gotten any attention so far, and we're trying to make it better.  But
> moving documents will, by its nature, annoy people.  We can generally get
> past that, but I'd really like to avoid moving things twice.  In general,
> I would rather see a single document converted, read critically and
> updated, and carefully integrated with the rest than a hundred of them
> swept into different piles...
> 
> See what I'm getting at?

I see what you mean, and I agree with this principle. That's basically 
why I split the patches into two groups. 

The first group (with comes first) does just the conversion
and renames from txt to rst, adding a :orphan: to the stuff that was
just converted.

On this series, those are patches 1 to 11. I was already expecting
some heat on patch 1.

The next group of patches do the renaming part. Those are the ones that
actually took me a lot more time, as I needed to quickly read several docs
in order to understand what's happening, before proposing a change.

That's also the group of patches were I expect more active comments,
as there are several cases where this is not obvious.

Yet, from what I saw, there are some documents that sounds easy to
move, like Documentation/laptops, with (except if I missed something)
clearly belongs to admin-guide.

Applying the second patch series and patches 2 to 11 from this third
series is, IMHO, a good thing to do.

-

IMO, patches 1 and 12 are important, as, after those patches, the
/Documentation dir becomes a lot cleaner:

	$ ls -F Documentation/
	ABI/              fb/              locking/        s390/
	accounting/       features/        logo.gif        scheduler/
	acpi/             filesystems/     logo.txt        scsi/
	admin-guide/      firmware_class/  m68k/           security/
	arm/              firmware-guide/  maintainer/     sh/
	arm64/            fpga/            Makefile        sound/
	auxdisplay/       gpio/            media/          sparc/
	block/            gpu/             mic/            sphinx/
	bpf/              hid/             mips/           sphinx-static/
	cdrom/            hwmon/           misc-devices/   spi/
	Changes@          i2c/             Module.symvers  SubmittingPatches
	CodingStyle       ia64/            netlabel/       target/
	conf.py           ide/             networking/     timers/
	core-api/         iio/             nios2/          trace/
	cpu-freq/         index.rst        openrisc/       translations/
	crypto/           infiniband/      output/         usb/
	devicetree/       input/           packing.txt     userspace-api/
	dev-tools/        ioctl/           parisc/         virtual/
	DocBook/          IPMB.txt         PCI/            vm/
	doc-guide/        isdn/            pcmcia/         w1/
	docutils.conf     kbuild/          power/          watchdog/
	dontdiff          Kconfig          powerpc/        wimax/
	driver-api/       kernel-hacking/  process/        x86/
	EDID/             leds/            RCU/            xtensa/
	fault-injection/  livepatch/       riscv/

Being easy to identify when someone tries to add a new text file there
without thinking on where it would fit[1], and to reorganize the
directory tree in a way that it will fit our needs.

[1] Btw, there are some two files at linux-next, incrementally
    increasing the Documentation/ mess:

	   IPMB.txt and packing.txt.

   Added on those commits:

	commit 51bd6f291583684f495ea498984dfc22049d7fd2
	Author: Asmaa Mnebhi <Asmaa@mellanox.com>
	Date:   Mon Jun 10 14:57:02 2019 -0400

	    Add support for IPMB driver

	commit 554aae35007e49f533d3d10e788295f7141725bc
	Author: Vladimir Oltean <olteanv@gmail.com>
	Date:   Thu May 2 23:23:29 2019 +0300

	    lib: Add support for generic packing operations

   We'll never finish organizing documents while people don't stop
   adding new files to Documentation/ directory.


Thanks,
Mauro

^ permalink raw reply

* Re: [PATCH v1 12/22] docs: driver-api: add .rst files from the main dir
From: Jani Nikula @ 2019-06-19 15:52 UTC (permalink / raw)
  To: Jonathan Corbet, Mauro Carvalho Chehab
  Cc: David Howells, Linux Doc Mailing List, Linux MM,
	Linux Kernel Mailing List
In-Reply-To: <20190619085458.08872dbb@lwn.net>

On Wed, 19 Jun 2019, Jonathan Corbet <corbet@lwn.net> wrote:
> Organization of the documentation tree is important; it has never really
> gotten any attention so far, and we're trying to make it better.  But
> moving documents will, by its nature, annoy people.  We can generally get
> past that, but I'd really like to avoid moving things twice.  In general,
> I would rather see a single document converted, read critically and
> updated, and carefully integrated with the rest than a hundred of them
> swept into different piles...

FWIW, as a first step, my preference would actually be cleaning up the
top level Documentation/ directory. Move every file to an existing or a
new subdirectory, even if just as .txt, or just delete. I understand
this would lead to an extra rst conversion and extension change later,
which you'd like to avoid, but IMO would be helpful.

We could even add an attic directory, which would be a suitable place
for things like zorro.txt. Attic is where I'd look for my old Amiga
hardware, so feels natural.

BR,
Jani.


-- 
Jani Nikula, Intel Open Source Graphics Center

^ permalink raw reply

* Re: [PATCH] docs: fb: Add TER16x32 to the available font names
From: Randy Dunlap @ 2019-06-19 15:25 UTC (permalink / raw)
  To: Takashi Iwai, Jonathan Corbet
  Cc: Bartlomiej Zolnierkiewicz, Greg Kroah-Hartman, linux-doc,
	linux-fbdev, linux-kernel
In-Reply-To: <20190619053943.6320-1-tiwai@suse.de>

On 6/18/19 10:39 PM, Takashi Iwai wrote:
> The new font is available since recently.
> 
> Signed-off-by: Takashi Iwai <tiwai@suse.de>

Acked-by: Randy Dunlap <rdunlap@infradead.org>

Thanks.

> ---
>  Documentation/fb/fbcon.rst | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Documentation/fb/fbcon.rst b/Documentation/fb/fbcon.rst
> index cfb9f7c38f18..1da65b9000de 100644
> --- a/Documentation/fb/fbcon.rst
> +++ b/Documentation/fb/fbcon.rst
> @@ -82,7 +82,7 @@ C. Boot options
>  
>  	Select the initial font to use. The value 'name' can be any of the
>  	compiled-in fonts: 10x18, 6x10, 7x14, Acorn8x8, MINI4x6,
> -	PEARL8x8, ProFont6x11, SUN12x22, SUN8x16, VGA8x16, VGA8x8.
> +	PEARL8x8, ProFont6x11, SUN12x22, SUN8x16, TER16x32, VGA8x16, VGA8x8.
>  
>  	Note, not all drivers can handle font with widths not divisible by 8,
>  	such as vga16fb.
> 


-- 
~Randy

^ permalink raw reply

* Re: [PATCH 1/6] docs: trace: fix a broken label
From: Masami Hiramatsu @ 2019-06-19 15:09 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Linux Doc Mailing List, Mauro Carvalho Chehab, linux-kernel,
	Jonathan Corbet, Masami Hiramatsu, Steven Rostedt (VMware),
	Andreas Ziegler, Lecopzer Chen
In-Reply-To: <a83ea390bc28784518fce772b4c961ea1c976f14.1560883872.git.mchehab+samsung@kernel.org>

On Tue, 18 Jun 2019 15:51:17 -0300
Mauro Carvalho Chehab <mchehab+samsung@kernel.org> wrote:

> Sphinx warnings about his:
> 
> 	Documentation/trace/kprobetrace.rst:68: WARNING: undefined label: user_mem_access (if the link has no caption the label must precede a section header)
> 
> The problem is quite simple: Sphinx wants a blank line after
> references.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>

This also looks good to me.

Acked-by: Masami Hiramatsu <mhiramat@kernel.org>

Thank you,

> ---
>  Documentation/trace/kprobetrace.rst | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/trace/kprobetrace.rst b/Documentation/trace/kprobetrace.rst
> index b729b40a5ba5..3d162d432a3c 100644
> --- a/Documentation/trace/kprobetrace.rst
> +++ b/Documentation/trace/kprobetrace.rst
> @@ -96,6 +96,7 @@ which shows given pointer in "symbol+offset" style.
>  For $comm, the default type is "string"; any other type is invalid.
>  
>  .. _user_mem_access:
> +
>  User Memory Access
>  ------------------
>  Kprobe events supports user-space memory access. For that purpose, you can use
> -- 
> 2.21.0
> 


-- 
Masami Hiramatsu <mhiramat@kernel.org>

^ permalink raw reply

* Re: [PATCH 2/6] docs: trace: add a missing blank line
From: Masami Hiramatsu @ 2019-06-19 15:08 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Linux Doc Mailing List, Mauro Carvalho Chehab, linux-kernel,
	Jonathan Corbet, Masami Hiramatsu, Steven Rostedt (VMware),
	Ingo Molnar, Andreas Ziegler, Lecopzer Chen
In-Reply-To: <91f90c10c12c6a2f6fb90fc0f9115fbd8dd73848.1560883872.git.mchehab+samsung@kernel.org>

On Tue, 18 Jun 2019 15:51:18 -0300
Mauro Carvalho Chehab <mchehab+samsung@kernel.org> wrote:

> Sphinx expects a blank line after a literal block markup.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>

Looks good to me, thanks!

Acked-by: Masami Hiramatsu <mhiramat@kernel.org>

> ---
>  Documentation/trace/kprobetrace.rst | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/trace/kprobetrace.rst b/Documentation/trace/kprobetrace.rst
> index 3d162d432a3c..caa0a8ba081e 100644
> --- a/Documentation/trace/kprobetrace.rst
> +++ b/Documentation/trace/kprobetrace.rst
> @@ -228,6 +228,7 @@ events, you need to enable it.
>  
>  Use the following command to start tracing in an interval.
>  ::
> +
>      # echo 1 > tracing_on
>      Open something...
>      # echo 0 > tracing_on
> -- 
> 2.21.0
> 


-- 
Masami Hiramatsu <mhiramat@kernel.org>

^ permalink raw reply

* Re: [PATCH v1 12/22] docs: driver-api: add .rst files from the main dir
From: Mauro Carvalho Chehab @ 2019-06-19 15:02 UTC (permalink / raw)
  To: Jonathan Corbet
  Cc: Peter Zijlstra, Linux Doc Mailing List, Linux Kernel Mailing List
In-Reply-To: <20190619081353.75762028@lwn.net>

Hi Jon,

Em Wed, 19 Jun 2019 08:13:53 -0600
Jonathan Corbet <corbet@lwn.net> escreveu:

> On Wed, 19 Jun 2019 12:42:39 +0200
> Peter Zijlstra <peterz@infradead.org> wrote:
> 
> > No, the other way around, Sphinx can recognize local files and treat
> > them special. That way we keep the text readable.
> > 
> > Same with that :c:func:'foo' crap, that needs to die, and Sphinx needs
> > to be taught about foo().  
> 
> I did a patch to make that latter part happen, but haven't been able to
> find the time to address the comments and get it out there.  It definitely
> cleaned up the source files a lot and is worth doing.  Will try to get
> back to it soon.

See my comment. Yeah, the :c:func:'foo' (the version you merged at the automarkup
branch) has currently a bug, when there's something like:

	func()
	======

or when func() is inside a table.

Solving the table case would be a lot better if the plugin could run the
existing table parser and only then handle the cross-reference replacements,
but I've no idea how flexible the Sphinx plugins can be.

> 
> The local file links should be easy to do; we shouldn't need to add any
> markup for those.

Yeah, those are easy - except if someone adds a Documentation/* link 
inside a table or inside a topic header.

Running a modified version of your tool shows just two new warnings:

	Documentation/translations/ja_JP/howto.rst:176: WARNING: undefined label: :doc: (if the link has no caption the label must precede a section header)         
	Documentation/translations/zh_CN/process/submitting-drivers.rst:25: WARNING: unknown document: ../../../Documentation/translations/zh_CN/process/submitting-patches

The first one is because of this:

	:Ref:`Documentation/process/kernel-docs.rst <kernel_docs>`

(my parser didn't consider upper-case tags - a simple fix at a regex should
fix this)

The second one is because the URL is wrong. It is pointing to:

	Documentation/Documentation/translations/zh_CN/process/submitting-patches

at Chinese translation.

So, at least the way our documentation is, the plugin seems to be working
as expected.

As a reference, I'm enclosing the diff against your patch:

    commit 6231d7456e87bd3e11f892709945887bd55a8a20 (docs/automarkup)
    Author: Jonathan Corbet <corbet@lwn.net>
    Date:   Thu Apr 25 07:55:07 2019 -0600

        Docs: An initial automarkup extension for sphinx
    
        Rather than fill our text files with :c:func:`function()` syntax, just do
        the markup via a hook into the sphinx build process.  As is always the
        case, the real problem is detecting the situations where this markup should
        *not* be done.
    
        Signed-off-by: Jonathan Corbet <corbet@lwn.net>

Thanks,
Mauro

-

diff --git a/Documentation/sphinx/automarkup.py b/Documentation/sphinx/automarkup.py
index 39c8f4d5af82..60dad596c790 100644
--- a/Documentation/sphinx/automarkup.py
+++ b/Documentation/sphinx/automarkup.py
@@ -9,6 +9,7 @@
 from __future__ import print_function
 import re
 import sphinx
+#import sys		# Just for debug
 
 #
 # Regex nastiness.  Of course.
@@ -31,10 +32,26 @@ RE_literal = re.compile(r'^(\s*)(.*::\s*|\.\.\s+code-block::.*)$')
 #
 RE_whitesp = re.compile(r'^(\s*)')
 
+#
+# Get a documentation reference
+#
+RE_doc_links = re.compile(r'\bDocumentation/([\w\d\-\_\/]+)\.rst\b')
+
+#
+# Doc link false-positives
+#
+RE_false_doc_links = re.compile(r':ref:`\s*Documentation/[\w\d\-\_\/]+\.rst')
+
 def MangleFile(app, docname, text):
     ret = [ ]
     previous = ''
     literal = False
+
+    rel_dir = ''
+
+    for depth in range(0, docname.count('/')):
+        rel_dir += "../"
+
     for line in text[0].split('\n'):
         #
         # See if we might be ending a literal block, as denoted by
@@ -63,7 +80,18 @@ def MangleFile(app, docname, text):
         # Normal line - perform substitutions.
         #
         else:
-            ret.append(RE_function.sub(r'\1:c:func:`\2`\3', line))
+#            new_line = RE_function.sub(r'\1:c:func:`\2`\3', line)
+            new_line = line
+
+            if not RE_false_doc_links.search(new_line):
+                new_line = RE_doc_links.sub(r':doc:`' + rel_dir + r'\1`', new_line)
+
+ #           # Just for debug - should be removed on production
+ #           if new_line != line:
+ #               print ("===>" + new_line, file=sys.stderr)
+
+            ret.append(new_line)
+
         #
         # Might we be starting a literal block?  If so make note of
         # the fact.



^ permalink raw reply related

* Re: [PATCH 04/14] ABI: better identificate tables
From: Greg Kroah-Hartman @ 2019-06-19 15:02 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Johan Hovold, Linux Doc Mailing List, Mauro Carvalho Chehab,
	Mauro Carvalho Chehab, linux-kernel, Jonathan Corbet,
	Stefan Achatz
In-Reply-To: <20190619105633.7f7315a5@coco.lan>

On Wed, Jun 19, 2019 at 10:56:33AM -0300, Mauro Carvalho Chehab wrote:
> Hi Johan,
> 
> Em Wed, 19 Jun 2019 14:51:35 +0200
> Johan Hovold <johan@kernel.org> escreveu:
> 
> > On Thu, Jun 13, 2019 at 11:04:10PM -0300, Mauro Carvalho Chehab wrote:
> > > From: Mauro Carvalho Chehab <mchehab@s-opensource.com>
> > > 
> > > When parsing via script, it is important to know if the script
> > > should consider a description as a literal block that should
> > > be displayed as-is, or if the description can be considered
> > > as a normal text.
> > > 
> > > Change descriptions to ensure that the preceding line of a table
> > > ends with a colon. That makes easy to identify the need of a
> > > literal block.  
> > 
> > In the cover letter you say that the first four patches of this series,
> > including this one, "fix some ABI descriptions that are violating the
> > syntax described at Documentation/ABI/README". This seems a bit harsh,
> > given that it's you that is now *introducing* a new syntax requirement
> > to assist your script.
> 
> Yeah, what's there at the cover letter doesn't apply to this specific
> patch. The thing is that I wrote this series a lot of time ago (2016/17).
> 
> I revived those per a request at KS ML, as we still need to expose the
> ABI content on some book that will be used by userspace people.
> 
> So, I just rebased it on the top of curent Kernel, add a cover letter
> with the things I remembered and re-sent.
> 
> In the specific case of this patch, the ":" there actually makes sense
> for someone that it is reading it as a text file, and it is an easy
> hack to make it parse better.
> 
> > Specifically, this new requirement isn't documented anywhere AFAICT, so
> > how will anyone adding new ABI descriptions learn about it?
> 
> Yeah, either that or provide an alternative to "Description" tag, to be
> used with more complex ABI descriptions.
> 
> One of the things that occurred to me, back on 2017, is that we should
> have a way to to specify that an specific ABI description would have
> a rich format. Something like:
> 
> What:		/sys/bus/usb/devices/<busnum>-<devnum>:<config num>.<interface num>/<hid-bus>:<vendor-id>:<product-id>.<num>/pyra/roccatpyra<minor>/actual_cpi
> Date:		August 2010
> Contact:	Stefan Achatz <erazor_de@users.sourceforge.net>
> RST-Description:
> 		It is possible to switch the cpi setting of the mouse with the
> 		press of a button.
> 		When read, this file returns the raw number of the actual cpi
> 		setting reported by the mouse. This number has to be further
> 		processed to receive the real dpi value:
> 
> 		===== =====
> 		VALUE DPI
> 		===== =====
> 		1     400
> 		2     800
> 		4     1600
> 		===== =====
> 
> With that, the script will know that the description contents will be using
> the ReST markup, and parse it accordingly. Right now, what it does, instead,
> is to place the description on a code-block, e. g. it will produce this
> output for the description:
> 
> ::
> 
> 		It is possible to switch the cpi setting of the mouse with the
> 		press of a button.
> 		When read, this file returns the raw number of the actual cpi
> 		setting reported by the mouse. This number has to be further
> 		processed to receive the real dpi value:
> 
> 		VALUE DPI
> 		1     400
> 		2     800
> 		4     1600
> 
> 
> Greg, 
> 
> what do you think?

I don't know when "Description" and "RST-Description" would be used.
Why not just parse "Description" like rst text and if things are "messy"
we fix them up as found, like you did with the ":" here?  It doesn't
have to be complex, we can always fix them up after-the-fact if new
stuff gets added that doesn't quite parse properly.

Just like we do for most kernel-doc formatting :)

thanks,

greg k-h

^ permalink raw reply

* Re: [PATCH v1 12/22] docs: driver-api: add .rst files from the main dir
From: Jonathan Corbet @ 2019-06-19 14:54 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: David Howells, Linux Doc Mailing List, Linux MM,
	Linux Kernel Mailing List
In-Reply-To: <20190619111528.3e2665e3@coco.lan>

[Trimming the CC list from hell made sense, but it might have been better
to leave me on it...]

On Wed, 19 Jun 2019 11:15:28 -0300
Mauro Carvalho Chehab <mchehab+samsung@kernel.org> wrote:

> Em Wed, 19 Jun 2019 14:39:10 +0100
> David Howells <dhowells@redhat.com> escreveu:
> 
> > Mauro Carvalho Chehab <mchehab+samsung@kernel.org> wrote:
> >   
> > > > > -Documentation/nommu-mmap.rst
> > > > > +Documentation/driver-api/nommu-mmap.rst      
> > 
> > Why is this moving to Documentation/driver-api?    
> 
> Good point. I tried to do my best with those document renames, but
> I'm pretty sure some of them ended by going to the wrong place - or
> at least there are arguments in favor of moving it to different
> places :-)

I think that a lot of this might also be an argument for slowing down just
a little bit.  I really don't think that blasting through and reformatting
all of our text documents is the most urgent problem right now and, in
cases like this, it might create others.

Organization of the documentation tree is important; it has never really
gotten any attention so far, and we're trying to make it better.  But
moving documents will, by its nature, annoy people.  We can generally get
past that, but I'd really like to avoid moving things twice.  In general,
I would rather see a single document converted, read critically and
updated, and carefully integrated with the rest than a hundred of them
swept into different piles...

See what I'm getting at?

Thanks,

jon

^ permalink raw reply

* [PATCH v2] dma-mapping: Fix filename references
From: Andy Shevchenko @ 2019-06-19 14:19 UTC (permalink / raw)
  To: Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86,
	H. Peter Anvin, Tony Luck, Fenghua Yu, Konrad Rzeszutek Wilk,
	linux-doc, Christoph Hellwig, Bjorn Helgaas
  Cc: Andy Shevchenko

After the commit cf65a0f6f6ff

  ("dma-mapping: move all DMA mapping code to kernel/dma")

some of the files are referring to outdated information, i.e. old file names
of DMA mapping sources.

Fix it here.

Note, the lines with "Glue code for..." have been removed completely.

Cc: Christoph Hellwig <hch@lst.de>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
- address Bjorn's comments
 Documentation/x86/x86_64/boot-options.rst | 2 +-
 arch/ia64/kernel/setup.c                  | 2 +-
 arch/x86/kernel/pci-swiotlb.c             | 1 -
 arch/x86/kernel/setup.c                   | 2 +-
 arch/x86/pci/sta2x11-fixup.c              | 4 +---
 5 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/Documentation/x86/x86_64/boot-options.rst b/Documentation/x86/x86_64/boot-options.rst
index 6a4285a3c7a4..2b98efb5ba7f 100644
--- a/Documentation/x86/x86_64/boot-options.rst
+++ b/Documentation/x86/x86_64/boot-options.rst
@@ -230,7 +230,7 @@ IOMMU (input/output memory management unit)
 ===========================================
 Multiple x86-64 PCI-DMA mapping implementations exist, for example:
 
-   1. <lib/dma-direct.c>: use no hardware/software IOMMU at all
+   1. <kernel/dma/direct.c>: use no hardware/software IOMMU at all
       (e.g. because you have < 3 GB memory).
       Kernel boot message: "PCI-DMA: Disabling IOMMU"
 
diff --git a/arch/ia64/kernel/setup.c b/arch/ia64/kernel/setup.c
index c9cfa760cd57..ab8d25d3e358 100644
--- a/arch/ia64/kernel/setup.c
+++ b/arch/ia64/kernel/setup.c
@@ -256,7 +256,7 @@ __initcall(register_memory);
  * This function checks if the reserved crashkernel is allowed on the specific
  * IA64 machine flavour. Machines without an IO TLB use swiotlb and require
  * some memory below 4 GB (i.e. in 32 bit area), see the implementation of
- * lib/swiotlb.c. The hpzx1 architecture has an IO TLB but cannot use that
+ * kernel/dma/swiotlb.c. The hpzx1 architecture has an IO TLB but cannot use that
  * in kdump case. See the comment in sba_init() in sba_iommu.c.
  *
  * So, the only machvec that really supports loading the kdump kernel
diff --git a/arch/x86/kernel/pci-swiotlb.c b/arch/x86/kernel/pci-swiotlb.c
index 5f5302028a9a..c2cfa5e7c152 100644
--- a/arch/x86/kernel/pci-swiotlb.c
+++ b/arch/x86/kernel/pci-swiotlb.c
@@ -1,5 +1,4 @@
 // SPDX-License-Identifier: GPL-2.0
-/* Glue code to lib/swiotlb.c */
 
 #include <linux/pci.h>
 #include <linux/cache.h>
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 08a5f4a131f5..8655bf374893 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -477,7 +477,7 @@ static int __init reserve_crashkernel_low(void)
 	ret = parse_crashkernel_low(boot_command_line, total_low_mem, &low_size, &base);
 	if (ret) {
 		/*
-		 * two parts from lib/swiotlb.c:
+		 * two parts from kernel/dma/swiotlb.c:
 		 * -swiotlb size: user-specified with swiotlb= or default.
 		 *
 		 * -swiotlb overflow buffer: now hardcoded to 32k. We round it
diff --git a/arch/x86/pci/sta2x11-fixup.c b/arch/x86/pci/sta2x11-fixup.c
index 97bbc12dd6b2..6269a175385d 100644
--- a/arch/x86/pci/sta2x11-fixup.c
+++ b/arch/x86/pci/sta2x11-fixup.c
@@ -1,8 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0-only
 /*
- * arch/x86/pci/sta2x11-fixup.c
- * glue code for lib/swiotlb.c and DMA translation between STA2x11
- * AMBA memory mapping and the X86 memory mapping
+ * DMA translation between STA2x11 AMBA memory mapping and the x86 memory mapping
  *
  * ST Microelectronics ConneXt (STA2X11/STA2X10)
  *
-- 
2.20.1


^ permalink raw reply related

* Re: [PATCH v1 12/22] docs: driver-api: add .rst files from the main dir
From: Mauro Carvalho Chehab @ 2019-06-19 14:15 UTC (permalink / raw)
  To: David Howells; +Cc: Linux Doc Mailing List, Linux MM, Linux Kernel Mailing List
In-Reply-To: <11422.1560951550@warthog.procyon.org.uk>

Hi David,

Em Wed, 19 Jun 2019 14:39:10 +0100
David Howells <dhowells@redhat.com> escreveu:

> Mauro Carvalho Chehab <mchehab+samsung@kernel.org> wrote:
> 
> > > > -Documentation/nommu-mmap.rst
> > > > +Documentation/driver-api/nommu-mmap.rst    
> 
> Why is this moving to Documentation/driver-api?  

Good point. I tried to do my best with those document renames, but
I'm pretty sure some of them ended by going to the wrong place - or
at least there are arguments in favor of moving it to different
places :-)

The driver-api ended to be where most of the stuff has been moved,
as this is the main kAPI dir (there is also a core-api dir for kAPI too).

I tend to place there stuff that has a mix of kAPI and uAPI at
driver-api, as usually such documents are written assuming that
the one that would be reading it is a Kernel developer.

> It's less to do with drivers
> than with the userspace mapping interface.  Documentation/vm/ would seem a
> better home.
> 
> Or should we institute a Documentation/uapi/?  Though that might be seen to
> overlap with man2.  Actually, should this be in man7?

Actually, there is an userspace-api dir too. While the logs show that
this was created back on 2017, I only noticed it very recently.

Re-checking the file, I see your point: there are lots of
userspace-relevant contents there. Yet, it also mentions kAPI internals,
like a reference for file and vm ops (at "Providing shareable character
device support" session):

	file->f_op->get_unmapped_area()
	file->f_op->mmap()
	vm_ops->close()

It is up to MM people and Jon to decide where to place it.

In any case, the best (long term) seems to split it on separate files, 
one with kAPI and another one with uAPI (just like may other subsystem
specific docs).

Thanks,
Mauro

^ permalink raw reply

* Re: [PATCH v1] dma-mapping: Fix filename references
From: Andy Shevchenko @ 2019-06-19 14:13 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86,
	H. Peter Anvin, Tony Luck, Fenghua Yu, Konrad Rzeszutek Wilk,
	linux-doc, Christoph Hellwig
In-Reply-To: <CAErSpo4v5qxza6Uyo8ZJ1kWWY2eBMxn5JQNQk2kAeZ2PZ2a+Yw@mail.gmail.com>

On Wed, Mar 20, 2019 at 04:31:17PM -0500, Bjorn Helgaas wrote:
> On Wed, Mar 20, 2019 at 11:13 AM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> >
> > After the commit cf65a0f6f6ff
> >
> >   ("dma-mapping: move all DMA mapping code to kernel/dma")
> >
> > some of the files are referring to outdated information, i.e. old file names
> > of DMA mapping sources.
> >
> > Fix it here.

Bjorn, thanks for review, my answers below.

> >   * This function checks if the reserved crashkernel is allowed on the specific
> >   * IA64 machine flavour. Machines without an IO TLB use swiotlb and require
> >   * some memory below 4 GB (i.e. in 32 bit area), see the implementation of
> > - * lib/swiotlb.c. The hpzx1 architecture has an IO TLB but cannot use that
> > + * kernel/dma/swiotlb.c. The hpzx1 architecture has an IO TLB but cannot use that
> >   * in kdump case. See the comment in sba_init() in sba_iommu.c.
> 
> Is the point here that just that if you lack an IOTLB and want devices
> to be able to reach system memory above 4GB, you need a bounce buffer
> below 4GB?  If so, maybe we could just say *that* instead of a
> nebulous reference to "the implementation of */swiotlb.c", which
> doesn't tell you what part of the implementation is relevant.

This patch is about broken links.

> > --- a/arch/x86/kernel/pci-swiotlb.c
> > +++ b/arch/x86/kernel/pci-swiotlb.c
> > @@ -1,5 +1,5 @@
> >  // SPDX-License-Identifier: GPL-2.0
> > -/* Glue code to lib/swiotlb.c */
> > +/* Glue code to kernel/dma/swiotlb.c */
> 
> I personally don't think there's much value in this line and I'd
> remove it entirely.

Will do.

> >                 /*
> > -                * two parts from lib/swiotlb.c:
> > +                * two parts from kernel/dma/swiotlb.c:
> >                  * -swiotlb size: user-specified with swiotlb= or default.
> >                  *
> >                  * -swiotlb overflow buffer: now hardcoded to 32k. We round it
> 
> Is there any chance this could be updated to refer to some variable or
> function names that grep/ctags/cscope/etc could find?  If we had that,
> I think the filename reference would be superfluous.

I'm not sure. Mauro probably the best person to ask what Sphinx can do here.

> >  /*
> >   * arch/x86/pci/sta2x11-fixup.c
> > - * glue code for lib/swiotlb.c and DMA translation between STA2x11
> > + * glue code for kernel/dma/swiotlb.c and DMA translation between STA2x11
> 
> I think both of these lines (the "arch/x86/pci/sta2x11-fixup.c" one
> and the "glue code" one) are superfluous.

Will remove.

> There's also a superfluous empty line at the end of the GPL license
> text, but I guess that should be removed by replacing the whole thing
> with an SPDX line.

Correct.

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply

* Re: [PATCH 04/14] ABI: better identificate tables
From: Mauro Carvalho Chehab @ 2019-06-19 13:56 UTC (permalink / raw)
  To: Johan Hovold, Greg Kroah-Hartman
  Cc: Linux Doc Mailing List, Mauro Carvalho Chehab,
	Mauro Carvalho Chehab, linux-kernel, Jonathan Corbet,
	Stefan Achatz
In-Reply-To: <20190619125135.GG25248@localhost>

Hi Johan,

Em Wed, 19 Jun 2019 14:51:35 +0200
Johan Hovold <johan@kernel.org> escreveu:

> On Thu, Jun 13, 2019 at 11:04:10PM -0300, Mauro Carvalho Chehab wrote:
> > From: Mauro Carvalho Chehab <mchehab@s-opensource.com>
> > 
> > When parsing via script, it is important to know if the script
> > should consider a description as a literal block that should
> > be displayed as-is, or if the description can be considered
> > as a normal text.
> > 
> > Change descriptions to ensure that the preceding line of a table
> > ends with a colon. That makes easy to identify the need of a
> > literal block.  
> 
> In the cover letter you say that the first four patches of this series,
> including this one, "fix some ABI descriptions that are violating the
> syntax described at Documentation/ABI/README". This seems a bit harsh,
> given that it's you that is now *introducing* a new syntax requirement
> to assist your script.

Yeah, what's there at the cover letter doesn't apply to this specific
patch. The thing is that I wrote this series a lot of time ago (2016/17).

I revived those per a request at KS ML, as we still need to expose the
ABI content on some book that will be used by userspace people.

So, I just rebased it on the top of curent Kernel, add a cover letter
with the things I remembered and re-sent.

In the specific case of this patch, the ":" there actually makes sense
for someone that it is reading it as a text file, and it is an easy
hack to make it parse better.

> Specifically, this new requirement isn't documented anywhere AFAICT, so
> how will anyone adding new ABI descriptions learn about it?

Yeah, either that or provide an alternative to "Description" tag, to be
used with more complex ABI descriptions.

One of the things that occurred to me, back on 2017, is that we should
have a way to to specify that an specific ABI description would have
a rich format. Something like:

What:		/sys/bus/usb/devices/<busnum>-<devnum>:<config num>.<interface num>/<hid-bus>:<vendor-id>:<product-id>.<num>/pyra/roccatpyra<minor>/actual_cpi
Date:		August 2010
Contact:	Stefan Achatz <erazor_de@users.sourceforge.net>
RST-Description:
		It is possible to switch the cpi setting of the mouse with the
		press of a button.
		When read, this file returns the raw number of the actual cpi
		setting reported by the mouse. This number has to be further
		processed to receive the real dpi value:

		===== =====
		VALUE DPI
		===== =====
		1     400
		2     800
		4     1600
		===== =====

With that, the script will know that the description contents will be using
the ReST markup, and parse it accordingly. Right now, what it does, instead,
is to place the description on a code-block, e. g. it will produce this
output for the description:

::

		It is possible to switch the cpi setting of the mouse with the
		press of a button.
		When read, this file returns the raw number of the actual cpi
		setting reported by the mouse. This number has to be further
		processed to receive the real dpi value:

		VALUE DPI
		1     400
		2     800
		4     1600


Greg, 

what do you think?

Thanks,
Mauro

^ permalink raw reply

* Re: [PATCH v1 12/22] docs: driver-api: add .rst files from the main dir
From: David Howells @ 2019-06-19 13:39 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: dhowells, Linux Doc Mailing List, Linux MM,
	Linux Kernel Mailing List
In-Reply-To: <20190619072218.4437f891@coco.lan>

Mauro Carvalho Chehab <mchehab+samsung@kernel.org> wrote:

> > > -Documentation/nommu-mmap.rst
> > > +Documentation/driver-api/nommu-mmap.rst  

Why is this moving to Documentation/driver-api?  It's less to do with drivers
than with the userspace mapping interface.  Documentation/vm/ would seem a
better home.

Or should we institute a Documentation/uapi/?  Though that might be seen to
overlap with man2.  Actually, should this be in man7?

David

^ permalink raw reply

* Re: [PATCH v1 12/22] docs: driver-api: add .rst files from the main dir
From: Mauro Carvalho Chehab @ 2019-06-19 13:19 UTC (permalink / raw)
  To: Peter Zijlstra, Linux Doc Mailing List, Jonathan Corbet,
	linux-kernel
In-Reply-To: <20190619114356.GP3419@hirez.programming.kicks-ass.net>

(c/c list cleaned)

Em Wed, 19 Jun 2019 13:43:56 +0200
Peter Zijlstra <peterz@infradead.org> escreveu:

> On Tue, Jun 18, 2019 at 05:53:17PM -0300, Mauro Carvalho Chehab wrote:
> 
> >  .../{ => driver-api}/atomic_bitops.rst        |  2 -  
> 
> That's a .txt file, big fat NAK for making it an rst.

Rst is a text file. This one is parsed properly by Sphinx without
any changes.

> 
> >  .../{ => driver-api}/futex-requeue-pi.rst     |  2 -  
> 
> >  .../{ => driver-api}/gcc-plugins.rst          |  2 -  
> 
> >  Documentation/{ => driver-api}/kprobes.rst    |  2 -
> >  .../{ => driver-api}/percpu-rw-semaphore.rst  |  2 -  
> 
> More NAK for rst conversion

Again, those don't need any conversion. Those files already parse 
as-is by Sphinx, with no need for any change.

The only change here is that, on patch 1/22, the files that
aren't listed on an index file got a :orphan: added in order
to make this explicit. This patch removes it.

> 
> >  Documentation/{ => driver-api}/pi-futex.rst   |  2 -
> >  .../{ => driver-api}/preempt-locking.rst      |  2 -  
> 
> >  Documentation/{ => driver-api}/rbtree.rst     |  2 -  
> 
> >  .../{ => driver-api}/robust-futex-ABI.rst     |  2 -
> >  .../{ => driver-api}/robust-futexes.rst       |  2 -  
> 
> >  .../{ => driver-api}/speculation.rst          |  8 +--
> >  .../{ => driver-api}/static-keys.rst          |  2 -  
> 
> >  .../{ => driver-api}/this_cpu_ops.rst         |  2 -  
> 
> >  Documentation/locking/rt-mutex.rst            |  2 +-  
> 
> NAK. None of the above have anything to do with driver-api.

Ok. Where do you think they should sit instead? core-api?

Thanks,
Mauro

^ permalink raw reply

* Re: [PATCH v1 12/22] docs: driver-api: add .rst files from the main dir
From: Mauro Carvalho Chehab @ 2019-06-19 13:04 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Linux Doc Mailing List, Linux Kernel Mailing List,
	Jonathan Corbet, Daniel Vetter
In-Reply-To: <20190619104239.GM3419@hirez.programming.kicks-ass.net>

Em Wed, 19 Jun 2019 12:42:39 +0200
Peter Zijlstra <peterz@infradead.org> escreveu:

(reduced the C/C to just the relevant ML/people)

> On Wed, Jun 19, 2019 at 07:22:18AM -0300, Mauro Carvalho Chehab wrote:
> > Hi Daniel,
> > 
> > Em Wed, 19 Jun 2019 11:05:57 +0200
> > Daniel Vetter <daniel@ffwll.ch> escreveu:
> >   
> > > On Tue, Jun 18, 2019 at 10:55 PM Mauro Carvalho Chehab
> > > <mchehab+samsung@kernel.org> wrote:  
> > > > diff --git a/Documentation/gpu/drm-mm.rst b/Documentation/gpu/drm-mm.rst
> > > > index fa30dfcfc3c8..b0f948d8733b 100644
> > > > --- a/Documentation/gpu/drm-mm.rst
> > > > +++ b/Documentation/gpu/drm-mm.rst
> > > > @@ -320,7 +320,7 @@ struct :c:type:`struct file_operations <file_operations>` get_unmapped_area
> > > >  field with a pointer on :c:func:`drm_gem_cma_get_unmapped_area`.
> > > >
> > > >  More detailed information about get_unmapped_area can be found in
> > > > -Documentation/nommu-mmap.rst
> > > > +Documentation/driver-api/nommu-mmap.rst    
> > > 
> > > Random drive-by comment: Could we convert these into hyperlinks within
> > > sphinx somehow, without making them less useful as raw file references
> > > (with vim I can just type 'gf' and it works, emacs probably the same).
> > > -Daniel  
> > 
> > Short answer: I don't know how vim/emacs would recognize Sphinx tags.  
> 
> No, the other way around, Sphinx can recognize local files and treat
> them special. That way we keep the text readable.
> 
> Same with that :c:func:'foo' crap, that needs to die, and Sphinx needs
> to be taught about foo().

Just did a test today with Jon's extension with is meant to replace
:c:func:'foo' (with is currently on a separate 'automarkup' branch).
At least the version therestill needs some work, as it currently 
mangles with titles and tables. like on those warnings/errors:

	6.4 :c:func:`resync_start()`, :c:func:`resync_finish()`
	-----------------------------------
	/devel/v4l/docs/Documentation/driver-api/md/md-cluster.rst:323: WARNING: Title underline too short.


	/devel/v4l/docs/Documentation/driver-api/serial/tty.rst:74: WARNING: Malformed table.
	Text in column margin in table line 34.

	======================= =======================================================
	:c:func:`open()`                        Called when the line discipline is attached to

-

That's said, once it gets fixed to address those complex cases, a
regex like:

	\bDocumentation/([\w\d\-\_\/]+)\.rst\b

could be used to convert to :doc: tag. It should be smart enough to convert
the relative paths, as we refer to the files from the git root directory
(with makes a lot sense to me), while Sphinx :doc: use relative patches
from the location where the parsed file is.

Something like the enclosed patch. 

Thanks,
Mauro

[PATCH] automarkup.py: convert Documentation/* to hyperlinks

Auto-create hyperlinks when it finds a Documentation/.. occurrence.

Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>

diff --git a/Documentation/sphinx/automarkup.py b/Documentation/sphinx/automarkup.py
index 39c8f4d5af82..9d6926b61241 100644
--- a/Documentation/sphinx/automarkup.py
+++ b/Documentation/sphinx/automarkup.py
@@ -9,6 +9,7 @@
 from __future__ import print_function
 import re
 import sphinx
+#import sys		# Just for debug
 
 #
 # Regex nastiness.  Of course.
@@ -31,10 +32,26 @@ RE_literal = re.compile(r'^(\s*)(.*::\s*|\.\.\s+code-block::.*)$')
 #
 RE_whitesp = re.compile(r'^(\s*)')
 
+#
+# Get a documentation reference
+#
+RE_doc_links = re.compile(r'\bDocumentation/([\w\d\-\_\/]+)\.rst\b')
+
+#
+# Doc link false-positives
+#
+RE_false_doc_links = re.compile(r':ref:`\s*Documentation/[\w\d\-\_\/]+\.rst')
+
 def MangleFile(app, docname, text):
     ret = [ ]
     previous = ''
     literal = False
+
+    rel_dir = ''
+
+    for depth in range(0, docname.count('/')):
+        rel_dir += "../"
+
     for line in text[0].split('\n'):
         #
         # See if we might be ending a literal block, as denoted by
@@ -63,7 +80,17 @@ def MangleFile(app, docname, text):
         # Normal line - perform substitutions.
         #
         else:
-            ret.append(RE_function.sub(r'\1:c:func:`\2`\3', line))
+            new_line = RE_function.sub(r'\1:c:func:`\2`\3', line)
+
+            if not RE_false_doc_links.search(new_line):
+                new_line = RE_doc_links.sub(r':doc:`' + rel_dir + r'\1`', new_line)
+
+ #           # Just for debug - should be removed on production
+ #           if new_line != line:
+ #               print ("===>" + new_line, file=sys.stderr)
+
+            ret.append(new_line)
+
         #
         # Might we be starting a literal block?  If so make note of
         # the fact.



^ permalink raw reply related

* Re: [PATCH 04/14] ABI: better identificate tables
From: Johan Hovold @ 2019-06-19 12:51 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Linux Doc Mailing List, Greg Kroah-Hartman, Mauro Carvalho Chehab,
	Mauro Carvalho Chehab, linux-kernel, Jonathan Corbet,
	Stefan Achatz
In-Reply-To: <6bc45c0d5d464d25d4d16eceac48a2f407166944.1560477540.git.mchehab+samsung@kernel.org>

On Thu, Jun 13, 2019 at 11:04:10PM -0300, Mauro Carvalho Chehab wrote:
> From: Mauro Carvalho Chehab <mchehab@s-opensource.com>
> 
> When parsing via script, it is important to know if the script
> should consider a description as a literal block that should
> be displayed as-is, or if the description can be considered
> as a normal text.
> 
> Change descriptions to ensure that the preceding line of a table
> ends with a colon. That makes easy to identify the need of a
> literal block.

In the cover letter you say that the first four patches of this series,
including this one, "fix some ABI descriptions that are violating the
syntax described at Documentation/ABI/README". This seems a bit harsh,
given that it's you that is now *introducing* a new syntax requirement
to assist your script.

Specifically, this new requirement isn't documented anywhere AFAICT, so
how will anyone adding new ABI descriptions learn about it?

> Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
> Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
> ---
>  Documentation/ABI/obsolete/sysfs-driver-hid-roccat-pyra   | 2 +-
>  .../ABI/testing/sysfs-class-backlight-driver-lm3533       | 6 +++---
>  Documentation/ABI/testing/sysfs-class-led-driver-lm3533   | 8 ++++----
>  Documentation/ABI/testing/sysfs-class-leds-gt683r         | 4 ++--
>  Documentation/ABI/testing/sysfs-driver-hid-roccat-kone    | 2 +-
>  5 files changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/Documentation/ABI/obsolete/sysfs-driver-hid-roccat-pyra b/Documentation/ABI/obsolete/sysfs-driver-hid-roccat-pyra
> index 16020b31ae64..5d41ebadf15e 100644
> --- a/Documentation/ABI/obsolete/sysfs-driver-hid-roccat-pyra
> +++ b/Documentation/ABI/obsolete/sysfs-driver-hid-roccat-pyra
> @@ -5,7 +5,7 @@ Description:	It is possible to switch the cpi setting of the mouse with the
>  		press of a button.
>  		When read, this file returns the raw number of the actual cpi
>  		setting reported by the mouse. This number has to be further
> -		processed to receive the real dpi value.
> +		processed to receive the real dpi value:
>  
>  		VALUE DPI
>  		1     400

Johan

^ permalink raw reply

* Re: [PATCH v1 14/22] docs: usb: rename files to .rst and add them to drivers-api
From: Johan Hovold @ 2019-06-19 12:39 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Linux Doc Mailing List, Mauro Carvalho Chehab, linux-kernel,
	Jonathan Corbet, Greg Kroah-Hartman, Oliver Neukum, Alan Stern,
	Johan Hovold, Valentina Manea, Shuah Khan, Felipe Balbi,
	linux-usb
In-Reply-To: <c05aecb424e4f835e3f7872ecb5818e1d2f3267c.1560891322.git.mchehab+samsung@kernel.org>

On Tue, Jun 18, 2019 at 06:05:38PM -0300, Mauro Carvalho Chehab wrote:
> While there are a mix of things here, most of the stuff
> were written from Kernel developer's PoV. So, add them to
> the driver-api book.
> 
> A follow up for this patch would be to move documents from
> there that are specific to sysadmins, adding them to the
> admin-guide.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>

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

^ permalink raw reply

* Re: [PATCH v5 1/2] fTPM: firmware TPM running in TEE
From: Ilias Apalodimas @ 2019-06-19 11:05 UTC (permalink / raw)
  To: Sasha Levin
  Cc: peterhuewe, jarkko.sakkinen, jgg, corbet, linux-kernel, linux-doc,
	linux-integrity, linux-kernel, thiruan, bryankel, tee-dev
In-Reply-To: <20190614202127.26812-2-sashal@kernel.org>

Hi Sasha, 

Can you please include me and Sumit in future iterations of this?
We've both commented and did some testing, we'd prefer not scanning the mailing
list for new versions.

[...]
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) Microsoft Corporation
> + *
> + * Implements a firmware TPM as described here:
> + * https://www.microsoft.com/en-us/research/publication/ftpm-software-implementation-tpm-chip/
> + *
> + * A reference implementation is available here:
> + * https://github.com/microsoft/ms-tpm-20-ref/tree/master/Samples/ARM32-FirmwareTPM/optee_ta/fTPM
> + */
> +
> +#include <linux/acpi.h>
> +#include <linux/of.h>
> +#include <linux/of_platform.h>
> +#include <linux/platform_device.h>
> +#include <linux/tee_drv.h>
> +#include <linux/tpm.h>
> +#include <linux/uuid.h>
> +
> +#include "tpm.h"
> +#include "tpm_ftpm_tee.h"
> +
> +#define DRIVER_NAME "ftpm-tee"
> +
> +/*
> + * TA_FTPM_UUID: BC50D971-D4C9-42C4-82CB-343FB7F37896
> + *
> + * Randomly generated, and must correspond to the GUID on the TA side.
> + * Defined here in the reference implementation:
> + * https://github.com/microsoft/ms-tpm-20-ref/blob/master/Samples/ARM32-FirmwareTPM/optee_ta/fTPM/include/fTPM.h#L42
> + */
> +
> +static const uuid_t ftpm_ta_uuid =
> +	UUID_INIT(0xBC50D971, 0xD4C9, 0x42C4,
> +		  0x82, 0xCB, 0x34, 0x3F, 0xB7, 0xF3, 0x78, 0x96);
> +
> +/**
> + * ftpm_tee_tpm_op_recv - retrieve fTPM response.
> + *
> + * @chip: the tpm_chip description as specified in driver/char/tpm/tpm.h.
> + * @buf: the buffer to store data.
> + * @count: the number of bytes to read.
> + *
> + * Return:
> + * 	In case of success the number of bytes received.
> + *	On failure, -errno.
> + */
> +static int ftpm_tee_tpm_op_recv(struct tpm_chip *chip, u8 *buf, size_t count)
> +{
> +	struct ftpm_tee_private *pvt_data = dev_get_drvdata(chip->dev.parent);
> +	size_t len;
> +
> +	len = pvt_data->resp_len;
> +	if (count < len) {
> +		dev_err(&chip->dev,
> +			"%s:Invalid size in recv: count=%zd, resp_len=%zd\n",
> +			__func__, count, len);
> +		return -EIO;
> +	}
> +
> +	memcpy(buf, pvt_data->resp_buf, len);
> +	pvt_data->resp_len = 0;
> +
> +	return len;
> +}
> +
> +/**
> + * ftpm_tee_tpm_op_send - send TPM commands through the TEE shared memory.
> + *
> + * @chip: the tpm_chip description as specified in driver/char/tpm/tpm.h
> + * @buf: the buffer to send.
> + * @len: the number of bytes to send.
> + *
> + * Return:
> + * 	In case of success, returns 0.
> + *	On failure, -errno
> + */
> +static int ftpm_tee_tpm_op_send(struct tpm_chip *chip, u8 *buf, size_t len)
> +{
> +	struct ftpm_tee_private *pvt_data = dev_get_drvdata(chip->dev.parent);
> +	size_t resp_len;
> +	int rc;
> +	u8 *temp_buf;
> +	struct tpm_header *resp_header;
> +	struct tee_ioctl_invoke_arg transceive_args;
> +	struct tee_param command_params[4];
> +	struct tee_shm *shm = pvt_data->shm;
> +
> +	if (len > MAX_COMMAND_SIZE) {
> +		dev_err(&chip->dev,
> +			"%s:len=%zd exceeds MAX_COMMAND_SIZE supported by fTPM TA\n",
> +			__func__, len);
> +		return -EIO;
> +	}
> +
> +	memset(&transceive_args, 0, sizeof(transceive_args));
> +	memset(command_params, 0, sizeof(command_params));
> +	pvt_data->resp_len = 0;
> +
> +	/* Invoke FTPM_OPTEE_TA_SUBMIT_COMMAND function of fTPM TA */
> +	transceive_args = (struct tee_ioctl_invoke_arg) {
> +		.func = FTPM_OPTEE_TA_SUBMIT_COMMAND,
> +		.session = pvt_data->session,
> +		.num_params = 4,
> +	};
> +
> +	/* Fill FTPM_OPTEE_TA_SUBMIT_COMMAND parameters */
> +	command_params[0] = (struct tee_param) {
> +		.attr = TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INPUT,
> +		.u.memref = {
> +			.shm = shm,
> +			.size = len,
> +			.shm_offs = 0,
> +		},
> +	};
> +
> +	temp_buf = tee_shm_get_va(shm, 0);
> +	if (IS_ERR(temp_buf)) {
> +		dev_err(&chip->dev, "%s:tee_shm_get_va failed for transmit\n",
> +			__func__);
> +		return PTR_ERR(temp_buf);
> +	}
> +	memset(temp_buf, 0, (MAX_COMMAND_SIZE + MAX_RESPONSE_SIZE));
> +
> +	memcpy(temp_buf, buf, len);
> +
> +	command_params[1] = (struct tee_param) {
> +		.attr = TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INOUT,
> +		.u.memref = {
> +			.shm = shm,
> +			.size = MAX_RESPONSE_SIZE,
> +			.shm_offs = MAX_COMMAND_SIZE,
> +		},
> +	};
> +
> +	rc = tee_client_invoke_func(pvt_data->ctx, &transceive_args,
> +					command_params);
> +	if ((rc < 0) || (transceive_args.ret != 0)) {
> +		dev_err(&chip->dev, "%s:SUBMIT_COMMAND invoke error: 0x%x\n",
> +			__func__, transceive_args.ret);
> +		return (rc < 0) ? rc : transceive_args.ret;
> +	}
> +
> +	temp_buf = tee_shm_get_va(shm, command_params[1].u.memref.shm_offs);
> +	if (IS_ERR(temp_buf)) {
> +		dev_err(&chip->dev, "%s:tee_shm_get_va failed for receive\n",
> +			__func__);
> +		return PTR_ERR(temp_buf);
> +	}
> +
> +	resp_header = (struct tpm_header *)temp_buf;
> +	resp_len = be32_to_cpu(resp_header->length);
> +
> +	/* sanity check resp_len */
> +	if (resp_len < TPM_HEADER_SIZE) {
> +		dev_err(&chip->dev, "%s:tpm response header too small\n",
> +			__func__);
> +		return -EIO;
> +	}
> +	if (resp_len > MAX_RESPONSE_SIZE) {
> +		dev_err(&chip->dev,
> +			"%s:resp_len=%zd exceeds MAX_RESPONSE_SIZE\n",
> +			__func__, resp_len);
> +		return -EIO;
> +	}
> +
> +	/* sanity checks look good, cache the response */
> +	memcpy(pvt_data->resp_buf, temp_buf, resp_len);
> +	pvt_data->resp_len = resp_len;
> +
> +	return 0;
> +}
> +
> +static void ftpm_tee_tpm_op_cancel(struct tpm_chip *chip)
> +{
> +	/* not supported */
> +}
> +
> +static u8 ftpm_tee_tpm_op_status(struct tpm_chip *chip)
> +{
> +	return 0;
> +}
> +
> +static bool ftpm_tee_tpm_req_canceled(struct tpm_chip *chip, u8 status)
> +{
> +	return 0;
> +}
> +
> +static const struct tpm_class_ops ftpm_tee_tpm_ops = {
> +	.flags = TPM_OPS_AUTO_STARTUP,
> +	.recv = ftpm_tee_tpm_op_recv,
> +	.send = ftpm_tee_tpm_op_send,
> +	.cancel = ftpm_tee_tpm_op_cancel,
> +	.status = ftpm_tee_tpm_op_status,
> +	.req_complete_mask = 0,
> +	.req_complete_val = 0,
> +	.req_canceled = ftpm_tee_tpm_req_canceled,
> +};
> +
> +/*
> + * Check whether this driver supports the fTPM TA in the TEE instance
> + * represented by the params (ver/data) to this function.
> + */
> +static int ftpm_tee_match(struct tee_ioctl_version_data *ver, const void *data)
> +{
> +	/*
> +	 * Currently this driver only support GP Complaint OPTEE based fTPM TA
> +	 */
> +	if ((ver->impl_id == TEE_IMPL_ID_OPTEE) &&
> +		(ver->gen_caps & TEE_GEN_CAP_GP))
> +		return 1;
> +	else
> +		return 0;
> +}
> +
> +/*
> + * Undo what has been done in ftpm_tee_probe
> + */
> +static void ftpm_tee_deinit(struct ftpm_tee_private *pvt_data)
I don't see this functions being used anywhere and my compiler complains. 
ftpm_tee_remove() is doing similar things, should this one be removed?

> +{
> +	/* Release the chip */
> +	tpm_chip_unregister(pvt_data->chip);
> +
> +	/* frees chip */
> +	if (pvt_data->chip)
> +		put_device(&pvt_data->chip->dev);
> +
> +	if (pvt_data->ctx) {
> +		/* Free the shared memory pool */
> +		tee_shm_free(pvt_data->shm);
> +
> +		/* close the existing session with fTPM TA*/
> +		tee_client_close_session(pvt_data->ctx, pvt_data->session);
> +
> +		/* close the context with TEE driver */
> +		tee_client_close_context(pvt_data->ctx);
> +	}
> +
> +	/* memory allocated with devm_kzalloc() is freed automatically */
> +}
> +
> +/**
> + * ftpm_tee_probe - initialize the fTPM
> + * @pdev: the platform_device description.
> + *
> + * Return:
> + * 	On success, 0. On failure, -errno.
> + */
> +static int ftpm_tee_probe(struct platform_device *pdev)
> +{
> +	int rc;
> +	struct tpm_chip *chip;
> +	struct device *dev = &pdev->dev;
> +	struct ftpm_tee_private *pvt_data = NULL;
> +	struct tee_ioctl_open_session_arg sess_arg;
> +
> +	pvt_data = devm_kzalloc(dev, sizeof(struct ftpm_tee_private),
> +				GFP_KERNEL);
> +	if (!pvt_data)
> +		return -ENOMEM;
> +
> +	dev_set_drvdata(dev, pvt_data);
> +
> +	/* Open context with TEE driver */
> +	pvt_data->ctx = tee_client_open_context(NULL, ftpm_tee_match, NULL,
> +						NULL);
> +	if (IS_ERR(pvt_data->ctx)) {
> +		if (ERR_PTR(pvt_data->ctx) == -ENOENT)
> +			return -EPROBE_DEFER;
> +		dev_err(dev, "%s:tee_client_open_context failed\n", __func__);
> +		return ERR_PTR(pvt_data->ctx);
> +	}
> +
> +	/* Open a session with fTPM TA */
> +	memset(&sess_arg, 0, sizeof(sess_arg));
> +	memcpy(sess_arg.uuid, ftpm_ta_uuid.b, TEE_IOCTL_UUID_LEN);
> +	sess_arg.clnt_login = TEE_IOCTL_LOGIN_PUBLIC;
> +	sess_arg.num_params = 0;
> +
> +	rc = tee_client_open_session(pvt_data->ctx, &sess_arg, NULL);
> +	if ((rc < 0) || (sess_arg.ret != 0)) {
> +		dev_err(dev, "%s:tee_client_open_session failed, err=%x\n",
> +			__func__, sess_arg.ret);
> +		rc = -EINVAL;
> +		goto out_tee_session;
> +	}
> +	pvt_data->session = sess_arg.session;
> +
> +	/* Allocate dynamic shared memory with fTPM TA */
> +	pvt_data->shm = tee_shm_alloc(pvt_data->ctx,
> +				(MAX_COMMAND_SIZE + MAX_RESPONSE_SIZE),
> +				TEE_SHM_MAPPED | TEE_SHM_DMA_BUF);
> +	if (IS_ERR(pvt_data->shm)) {
> +		dev_err(dev, "%s:tee_shm_alloc failed\n", __func__);
> +		rc = -ENOMEM;
> +		goto out_shm_alloc;
> +	}
> +
> +	/* Allocate new struct tpm_chip instance */
> +	chip = tpm_chip_alloc(dev, &ftpm_tee_tpm_ops);
> +	if (IS_ERR(chip)) {
> +		dev_err(dev, "%s:tpm_chip_alloc failed\n", __func__);
> +		rc = PTR_ERR(chip);
> +		goto out_chip_alloc;
> +	}
> +
> +	pvt_data->chip = chip;
> +	pvt_data->chip->flags |= TPM_CHIP_FLAG_TPM2;
> +
> +	/* Create a character device for the fTPM */
> +	rc = tpm_chip_register(pvt_data->chip);
> +	if (rc) {
> +		dev_err(dev, "%s:tpm_chip_register failed with rc=%d\n",
> +			__func__, rc);
> +		goto out_chip;
> +	}
> +
> +	return 0;
> +
> +out_chip:
> +	put_device(&pvt_data->chip->dev);
> +out_chip_alloc:
> +	tee_shm_free(pvt_data->shm);
> +out_shm_alloc:
> +	tee_client_close_session(pvt_data->ctx, pvt_data->session);
> +out_tee_session:
> +	tee_client_close_context(pvt_data->ctx);
> +
> +	return rc;
> +}
> +
> +/**
> + * ftpm_tee_remove - remove the TPM device
> + * @pdev: the platform_device description.
> + *
> + * Return:
> + * 	0 in case of success.
> + */
> +static int ftpm_tee_remove(struct platform_device *pdev)
> +{
> +	struct ftpm_tee_private *pvt_data = dev_get_drvdata(&pdev->dev);
> +
> +	/* Release the chip */
> +	tpm_chip_unregister(pvt_data->chip);
> +
> +	/* frees chip */
> +	put_device(&pvt_data->chip->dev);
> +
> +	/* Free the shared memory pool */
> +	tee_shm_free(pvt_data->shm);
> +
> +	/* close the existing session with fTPM TA*/
> +	tee_client_close_session(pvt_data->ctx, pvt_data->session);
> +
> +	/* close the context with TEE driver */
> +	tee_client_close_context(pvt_data->ctx);
> +
> +        /* memory allocated with devm_kzalloc() is freed automatically */
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id of_ftpm_tee_ids[] = {
> +	{ .compatible = "microsoft,ftpm" },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, of_ftpm_tee_ids);
> +
> +static struct platform_driver ftpm_tee_driver = {
> +	.driver = {
> +		.name = DRIVER_NAME,
> +		.of_match_table = of_match_ptr(of_ftpm_tee_ids),
> +	},
> +	.probe = ftpm_tee_probe,
> +	.remove = ftpm_tee_remove,
> +};
> +
> +module_platform_driver(ftpm_tee_driver);
> +
> +MODULE_AUTHOR("Thirupathaiah Annapureddy <thiruan@microsoft.com>");
> +MODULE_DESCRIPTION("TPM Driver for fTPM TA in TEE");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/char/tpm/tpm_ftpm_tee.h b/drivers/char/tpm/tpm_ftpm_tee.h
> new file mode 100644
> index 000000000000..b09ee7be4545
> --- /dev/null
> +++ b/drivers/char/tpm/tpm_ftpm_tee.h
> @@ -0,0 +1,40 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (C) Microsoft Corporation
> + */
> +
> +#ifndef __TPM_FTPM_TEE_H__
> +#define __TPM_FTPM_TEE_H__
> +
> +#include <linux/tee_drv.h>
> +#include <linux/tpm.h>
> +#include <linux/uuid.h>
> +
> +/* The TAFs ID implemented in this TA */
> +#define FTPM_OPTEE_TA_SUBMIT_COMMAND  (0)
> +#define FTPM_OPTEE_TA_EMULATE_PPI     (1)
> +
> +/* max. buffer size supported by fTPM  */
> +#define  MAX_COMMAND_SIZE       4096
> +#define  MAX_RESPONSE_SIZE      4096
> +
> +/**
> + * struct ftpm_tee_private - fTPM's private data
> + * @chip:     struct tpm_chip instance registered with tpm framework.
> + * @state:    internal state
> + * @session:  fTPM TA session identifier.
> + * @resp_len: cached response buffer length.
> + * @resp_buf: cached response buffer.
> + * @ctx:      TEE context handler.
> + * @shm:      Memory pool shared with fTPM TA in TEE.
> + */
> +struct ftpm_tee_private {
> +	struct tpm_chip *chip;
> +	u32 session;
> +	size_t resp_len;
> +	u8 resp_buf[MAX_RESPONSE_SIZE];
> +	struct tee_context *ctx;
> +	struct tee_shm *shm;
> +};
> +
> +#endif /* __TPM_FTPM_TEE_H__ */
> -- 
> 2.20.1
> 

Regards
/Ilias

^ permalink raw reply

* Re: [PATCH v1 20/22] docs: extcon: move it to acpi dir and convert it to ReST
From: Mauro Carvalho Chehab @ 2019-06-19 10:43 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Linux Doc Mailing List, Mauro Carvalho Chehab, linux-kernel,
	Jonathan Corbet, MyungJoo Ham, Chanwoo Choi, Len Brown,
	linux-acpi
In-Reply-To: <4701210.Ilfu9VLqBR@kreacher>

Em Wed, 19 Jun 2019 11:59:18 +0200
"Rafael J. Wysocki" <rjw@rjwysocki.net> escreveu:

> On Tuesday, June 18, 2019 11:05:44 PM CEST Mauro Carvalho Chehab wrote:
> > The intel-int3496.txt file is a documentation for an ACPI driver.
> > 
> > There's no reason to keep it on a separate directory.
> > 
> > So, instead of keeping it on some random location, move it
> > to a sub-directory inside the ACPI documentation dir.
> > 
> > For now, keep it with .txt extension, in order to avoid
> > Sphinx build noise. A later patch should change it to .rst
> > and movin it to be together with other acpi docs.
> > 
> > Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>  
> 
> Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> Or please let me know if you want me to pick up this one.

Feel free to pick it.


Thanks,
Mauro

^ permalink raw reply

* Re: [PATCH v1 20/22] docs: extcon: move it to acpi dir and convert it to ReST
From: Rafael J. Wysocki @ 2019-06-19 10:02 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Linux Doc Mailing List, Mauro Carvalho Chehab,
	Linux Kernel Mailing List, Jonathan Corbet, MyungJoo Ham,
	Chanwoo Choi, Len Brown, ACPI Devel Maling List
In-Reply-To: <4701210.Ilfu9VLqBR@kreacher>

On Wed, Jun 19, 2019 at 11:59 AM Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
>
> On Tuesday, June 18, 2019 11:05:44 PM CEST Mauro Carvalho Chehab wrote:
> > The intel-int3496.txt file is a documentation for an ACPI driver.
> >
> > There's no reason to keep it on a separate directory.
> >
> > So, instead of keeping it on some random location, move it
> > to a sub-directory inside the ACPI documentation dir.
> >
> > For now, keep it with .txt extension, in order to avoid
> > Sphinx build noise. A later patch should change it to .rst
> > and movin it to be together with other acpi docs.
> >
> > Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
>
> Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Which said, the changelog appears to be outdated, because the format
of the doc *does* change.

> > ---
> >  .../acpi/extcon-intel-int3496.rst}                 | 14 ++++++++++----
> >  Documentation/firmware-guide/acpi/index.rst        |  1 +
> >  MAINTAINERS                                        |  6 +++---
> >  3 files changed, 14 insertions(+), 7 deletions(-)
> >  rename Documentation/{extcon/intel-int3496.txt => firmware-guide/acpi/extcon-intel-int3496.rst} (66%)
> >
> > diff --git a/Documentation/extcon/intel-int3496.txt b/Documentation/firmware-guide/acpi/extcon-intel-int3496.rst

^ permalink raw reply


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